Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f0c569d

Browse files
committed
Fix memory leak in pgoutput with publication list cache
The pgoutput module caches publication names in a list and frees it upon invalidation. However, the code forgot to free the actual publication names within the list elements, as publication names are pstrdup()'d in GetPublication(). This would cause memory to leak in CacheMemoryContext, bloating it over time as this context is not cleaned. This is a problem for WAL senders running for a long time, as an accumulation of invalidation requests would bloat its cache memory usage. A second case, where this leak is easier to see, involves a backend calling SQL functions like pg_logical_slot_{get,peek}_changes() which create a new decoding context with each execution. More publications create more bloat. To address this, this commit adds a new memory context within the logical decoding context and resets it each time the publication names cache is invalidated, based on a suggestion from Amit Kapila. This ensures that the lifespan of the publication names aligns with that of the logical decoding context. This solution changes PGOutputData, which is fine for HEAD but it could cause an ABI breakage in stable branches as the structure size would change, so these are left out for now. Analyzed-by: Michael Paquier, Jeff Davis Author: Zhijie Hou Reviewed-by: Michael Paquier, Masahiko Sawada, Euler Taveira Discussion: https://postgr.es/m/Z0khf9EVMVLOc_YY@paquier.xyz
1 parent 001a537 commit f0c569d

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

src/backend/replication/pgoutput/pgoutput.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,10 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
436436
"logical replication cache context",
437437
ALLOCSET_DEFAULT_SIZES);
438438

439+
data->pubctx = AllocSetContextCreate(ctx->context,
440+
"logical replication publication list context",
441+
ALLOCSET_SMALL_SIZES);
442+
439443
ctx->output_plugin_private = data;
440444

441445
/* This plugin uses binary protocol. */
@@ -1734,9 +1738,9 @@ pgoutput_origin_filter(LogicalDecodingContext *ctx,
17341738
/*
17351739
* Shutdown the output plugin.
17361740
*
1737-
* Note, we don't need to clean the data->context and data->cachectx as
1738-
* they are child contexts of the ctx->context so they will be cleaned up by
1739-
* logical decoding machinery.
1741+
* Note, we don't need to clean the data->context, data->cachectx, and
1742+
* data->pubctx as they are child contexts of the ctx->context so they
1743+
* will be cleaned up by logical decoding machinery.
17401744
*/
17411745
static void
17421746
pgoutput_shutdown(LogicalDecodingContext *ctx)
@@ -2063,12 +2067,9 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
20632067
/* Reload publications if needed before use. */
20642068
if (!publications_valid)
20652069
{
2066-
oldctx = MemoryContextSwitchTo(CacheMemoryContext);
2067-
if (data->publications)
2068-
{
2069-
list_free_deep(data->publications);
2070-
data->publications = NIL;
2071-
}
2070+
MemoryContextReset(data->pubctx);
2071+
2072+
oldctx = MemoryContextSwitchTo(data->pubctx);
20722073
data->publications = LoadPublications(data->publication_names);
20732074
MemoryContextSwitchTo(oldctx);
20742075
publications_valid = true;

src/include/replication/pgoutput.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ typedef struct PGOutputData
2020
MemoryContext context; /* private memory context for transient
2121
* allocations */
2222
MemoryContext cachectx; /* private memory context for cache data */
23+
MemoryContext pubctx; /* private memory context for publication data */
2324

2425
bool in_streaming; /* true if we are streaming a chunk of
2526
* transaction */

0 commit comments

Comments
 (0)