public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Shinya Kato <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: cca5507 <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Call EndCopyFrom() after initial table sync in logical replication
Date: Sat, 9 May 2026 13:57:10 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOzEurTDq0_jDnFo64-r-ggt=ykXv-rhotcGDvjaktw+O6VV1g@mail.gmail.com>
References: <CAOzEurQKuy3RiPkd=25PEwEzaqHuGvEOf=X7vaVzhgNjaukYzA@mail.gmail.com>
	<CAHGQGwEpwuq9U=Hhx3BbfGY-RrJ5rFvdQopO+0xyAuLVsMdfWA@mail.gmail.com>
	<[email protected]>
	<CAHGQGwHfdbfRRVdQjYRZmT94UMqE1W4MENiaseVtsHWmzkWxBQ@mail.gmail.com>
	<[email protected]>
	<CAOzEurTDq0_jDnFo64-r-ggt=ykXv-rhotcGDvjaktw+O6VV1g@mail.gmail.com>



> On May 9, 2026, at 02:05, Shinya Kato <[email protected]> wrote:
> 
> 
> 
> On Fri, May 8, 2026, 14:10 Chao Li <[email protected]> wrote:
> 
> I don’t 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.
> 
> I agree that no additional memory cleanup is　needed here.
> 
> 
> While tracing the code, I noticed another issue that is probably more worth addressing. copy_table() currently does:
> ```
>     copybuf = makeStringInfo();
> ```
> 
> 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.
> 
> 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.
> 
> 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.
> 
> Thanks for the suggestion.
> 
> The copybuf change looks worthwhile, but perhaps it’s better discussed in a separate thread.
> 

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

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

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Call EndCopyFrom() after initial table sync in logical replication
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox