Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Call EndCopyFrom() after initial table sync in logical
 replication
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAOzEurTDq0_jDnFo64-r-ggt=ykXv-rhotcGDvjaktw+O6VV1g@mail.gmail.com>
Date: Sat, 9 May 2026 13:57:10 +0800
Cc: Fujii Masao <masao.fujii@gmail.com>,
 cca5507 <cca5507@qq.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <140BF04C-441C-451A-80E7-2C0BD585FBFA@gmail.com>
References: 
 <CAOzEurQKuy3RiPkd=25PEwEzaqHuGvEOf=X7vaVzhgNjaukYzA@mail.gmail.com>
 <CAHGQGwEpwuq9U=Hhx3BbfGY-RrJ5rFvdQopO+0xyAuLVsMdfWA@mail.gmail.com>
 <tencent_B56532E9785E072776FBA169850EB5032908@qq.com>
 <CAHGQGwHfdbfRRVdQjYRZmT94UMqE1W4MENiaseVtsHWmzkWxBQ@mail.gmail.com>
 <E6D725B9-1897-498A-BFDC-A86B84E03546@gmail.com>
 <CAOzEurTDq0_jDnFo64-r-ggt=ykXv-rhotcGDvjaktw+O6VV1g@mail.gmail.com>
To: Shinya Kato <shinya11.kato@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/140BF04C-441C-451A-80E7-2C0BD585FBFA%40gmail.com>
Precedence: bulk


> On May 9, 2026, at 02:05, Shinya Kato <shinya11.kato@gmail.com> wrote:
>=20
>=20
>=20
> On Fri, May 8, 2026, 14:10 Chao Li <li.evan.chao@gmail.com> wrote:
>=20
> I don=E2=80=99t think this is a serious leak. In this path, pstate and =
attnamelist are allocated in CurTransactionContext, and the transaction =
is committed immediately after copy_table() finishes, so that memory is =
reclaimed at transaction end. Explicitly freeing them would be mostly =
for code readability, not to fix a memory leak. So, I am okay to not =
free them.
>=20
> I agree that no additional memory cleanup is=E3=80=80needed here.
>=20
>=20
> While tracing the code, I noticed another issue that is probably more =
worth addressing. copy_table() currently does:
> ```
>     copybuf =3D makeStringInfo();
> ```
>=20
> But copybuf is only used by copy_read_data(), and there it's really =
just acting as a small state holder for data, len, and cursor, rather =
than as a normal growable StringInfo. That means we do not need to =
allocate a StringInfo object or its backing buffer at all.
>=20
> It would be cleaner to use a plain StringInfoData and simply =
reinitialize or zero it in copy_table(). See the attached diff for the =
proposed change.
>=20
> David Rowley has made several cleanup changes in this area to prefer =
stack-allocated StringInfoData, for example =
a63bbc811d41b3567eb37fe2636e660a852dbbf2. This change seems consistent =
with that direction.
>=20
> Thanks for the suggestion.
>=20
> The copybuf change looks worthwhile, but perhaps it=E2=80=99s better =
discussed in a separate thread.
>=20

Sound fair. Let me post it to a separate thread.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/