Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Michael Paquier <michael@paquier.xyz>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-12-03T04:20:42Z
Lists: pgsql-hackers 
On Tue, Dec 3, 2024 at 2:12 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Dec-02, Amit Kapila wrote:
>
> > Even, if we want a new context for some localized handling, we should
> > add that in PGOutputData rather than a local context as the proposed
> > patch is doing at the very least for HEAD.
>
> I don't necessarily agree, given that this context is not needed
> anywhere else.
>

But that suits the current design more. We allocate PGOutputData and
other contexts in that structure in a "Logical decoding context". A
few of its members (publications, publication_names) residing in
totally unrelated contexts sounds odd. In the first place, we don't
need to allocate publications under CacheMemoryContext, they should be
allocated in PGOutputData->cachectx. However, because we need to free
those entirely at one-shot during invalidation processing, we could
use a new context as a child context of PGOutputData->cachectx. Unless
I am missing something, the current memory context usage appears more
like a coding convenience than a thoughtful design decision.

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix memory leak in pgoutput with relation attribute map

  2. Fix memory leak in pgoutput with publication list cache

  3. Allow specifying row filters for logical replication of tables.