database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-06 19:59:18 +03:00
Code

Fix memory leak in pgoutput with publication list cache

Browse Source 
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.

Contrary to the HEAD-only commit f0c569d71515 that has changed
PGOutputData to track this new child memory context, the context is
tracked with a static variable whose state is reset with a MemoryContext
reset callback attached to PGOutputData->context, so as ABI
compatibility is preserved in stable branches.  This approach is based
on an suggestion from Amit Kapila.

Analyzed-by: Michael Paquier, Jeff Davis
Author: Masahiko Sawada
Reviewed-by: Amit Kapila, Michael Paquier, Euler Taveira, Hou Zhijie
Discussion: https://postgr.es/m/Z0khf9EVMVLOc_YY@paquier.xyz
Backpatch-through: 13
This commit is contained in:
Michael Paquier 2024-12-23 12:48:06 +09:00
parent 7cfdb4d1e7
commit bbe68c13ab
1 changed files with 37 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/backend/replication/pgoutput/pgoutput.c
View File

@ -81,6 +81,13 @@ static void pgoutput_stream_prepare_txn(LogicalDecodingContext *ctx,
static bool publications_valid;
/*
* Private memory context for publication data, created in
* PGOutputData->context when starting pgoutput, and set to NULL when its
* parent context is reset via a dedicated MemoryContextCallback.
*/
static MemoryContext pubctx = NULL;
static List *LoadPublications(List *pubnames);
static void publication_invalidation_cb(Datum arg, int cacheid,
uint32 hashvalue);
@ -411,6 +418,15 @@ parse_output_parameters(List *options, PGOutputData *data)
errmsg("option \"%s\" missing", "publication_names"));
}
/*
* Callback of PGOutputData->context in charge of cleaning pubctx.
*/
static void
pgoutput_pubctx_reset_callback(void *arg)
{
pubctx = NULL;
}
/*
* Initialize this plugin
*/
@ -420,6 +436,7 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
{
PGOutputData *data = palloc0(sizeof(PGOutputData));
static bool publication_callback_registered = false;
MemoryContextCallback *mcallback;
/* Create our memory context for private allocations. */
data->context = AllocSetContextCreate(ctx->context,
@ -430,6 +447,15 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
"logical replication cache context",
ALLOCSET_DEFAULT_SIZES);
Assert(pubctx == NULL);
pubctx = AllocSetContextCreate(ctx->context,
"logical replication publication list context",
ALLOCSET_SMALL_SIZES);
mcallback = palloc0(sizeof(MemoryContextCallback));
mcallback->func = pgoutput_pubctx_reset_callback;
MemoryContextRegisterResetCallback(ctx->context, mcallback);
ctx->output_plugin_private = data;
/* This plugin uses binary protocol. */
@ -1696,9 +1722,9 @@ pgoutput_origin_filter(LogicalDecodingContext *ctx,
/*
* Shutdown the output plugin.
*
* Note, we don't need to clean the data->context and data->cachectx as
* they are child contexts of the ctx->context so they will be cleaned up by
* logical decoding machinery.
* Note, we don't need to clean the data->context, data->cachectx and pubctx
* as they are child contexts of the ctx->context so they will be cleaned up
* by logical decoding machinery.
*/
static void
pgoutput_shutdown(LogicalDecodingContext *ctx)
@ -1708,6 +1734,9 @@ pgoutput_shutdown(LogicalDecodingContext *ctx)
hash_destroy(RelationSyncCache);
RelationSyncCache = NULL;
}
/* Better safe than sorry */
pubctx = NULL;
}
/*
@ -2024,12 +2053,11 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
/* Reload publications if needed before use. */
if (!publications_valid)
{
oldctx = MemoryContextSwitchTo(CacheMemoryContext);
if (data->publications)
{
list_free_deep(data->publications);
data->publications = NIL;
}
Assert(pubctx);
MemoryContextReset(pubctx);
oldctx = MemoryContextSwitchTo(pubctx);
data->publications = LoadPublications(data->publication_names);
MemoryContextSwitchTo(oldctx);
publications_valid = true;