Horton

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

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Euler Taveira <euler@eulerto.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-12-09T23:35:21Z
Lists: pgsql-hackers 
On Mon, Dec 09, 2024 at 12:46:35PM -0800, Masahiko Sawada wrote:
> True. There seems another place where we possibly leak memory on
> CacheMemoryContext when using pgoutput via SQL APIs:
> 
>         /* Map must live as long as the session does. */
>         oldctx = MemoryContextSwitchTo(CacheMemoryContext);
> 
>         entry->attrmap = build_attrmap_by_name_if_req(indesc, outdesc, false);
> 
>         MemoryContextSwitchTo(oldctx);
>         RelationClose(ancestor);
> 
> entry->attrmap is pfree'd only when validating the RelationSyncEntry
> so remains even after logical decoding API calls.

Right.  I'm also slightly worried about how we handle streamed_txns in
set_schema_sent_in_streamed_txn() through CacheMemoryContext.  It
feels like this could be made more robust without relying on an
explicit list_free() in get_rel_sync_entry(), including the fact that
some cleanup also relies on pgoutput_stream_abort() being taken.

As a whole, thinking long-term, it seems to me that we'd live better
if we remove all direct dependencies to CacheMemoryContext in this
code.
--
Michael

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.