Feedback-ID: ia2694551:Fastmail
Date: Tue, 3 Dec 2024 14:45:22 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, 
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Memory leak in WAL sender with pgoutput (v10~)
Message-ID: <202412031345.jxrlhwe2a2qa@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAA4eK1+TCM5mqVXG-6VXoptFdc5c2+OMYABrpr4fc1r5f5M0yw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/202412031345.jxrlhwe2a2qa%40alvherre.pgsql>
Precedence: bulk

On 2024-Dec-03, Amit Kapila wrote:

> On Tue, Dec 3, 2024 at 4:03 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > If you don't like the idea of a static memcxt in the one block where
> > it's needed, I propose to store a new memcxt in PGOutputData, to be used
> > exclusively for publications, with a well defined lifetime.
> 
> +1. This sounds like a way to proceed at least for HEAD. For
> back-branches, it is less clear whether changing PGOutputData is a
> good idea. Can such a change in back branches break any existing
> non-core code (extensions)?

We can put the new member at the end of the struct, it shouldn't damage
anything even if they're using this struct -- which I find pretty
unlikely.  The only way that could break anything is if somebody is
allocating/using arrays of it, which sounds even more unlikely.

If we don't want to accept that risk (for which I see no argument, but
happy to be proven wrong), I would suggest to use the foreach-pfree
pattern Michael first proposed for the backbranches, and the new memory
context in master.  I think this is conducive to better coding overall
as we clean things up in this area.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/