RE: Memory leak in WAL sender with pgoutput (v10~)

On Wednesday, December 4, 2024 8:55 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Dec 03, 2024 at 09:46:06PM -0300, Euler Taveira wrote:
> > Although, Debian code search [1] says this data structure is not used
> outside
> > PostgreSQL, I wouldn't risk breaking third-party extensions during a minor
> > upgrade (even if it is known that such data structure is from that particular
> > output plugin -- pgoutput -- and other output plugins generally have its own
> > data structure). +1 from Alvaro's proposal.
>
> A lookup of the public repos of github did not show fancy with the
> manipulation of the structure for peoject related to Postgres, either.
>
> FWIW, I'm OK with the memory context reset solution as much as the
> direct free calls as we are sure that they will be safe.  And at the
> end of the day, the problem would be solved with any of these
> solutions.  My votes would be +0.6 for the free and +0.5 for the mcxt
> manipulation, so let's say that they are tied.
>
> As Alvaro and yourself are in favor of the mcxt approach, then let's
> go for it.

+1

> Amit has concerns with other code paths that could be
> similarly leaking.  I'm not sure if this is worth waiting too long
> based on how local the fix for the existing leak is with any of these
> solutions.

It appears there is an additional memory leak caused by allocating publication
names within the CacheMemoryContext, as noted in [1]. And it can also be fixed by
creating a separate memctx for publication names under the logical decoding
context. I think the approach makes sense since the lifespan of publication
names should ideally align with that of the logical decoding context.

[1]
https://www.postgresql.org/message-id/OS0PR01MB57166A4DA0ABBB94F2FBB28694362%40OS0PR01MB5716.jpnprd01.prod.outlook.com

Best Regards,
Hou zj