v1-0001-Fix-memory-leak-in-pgoutput-with-static-memory-co_PG17.patch

text/x-patch

Filename: v1-0001-Fix-memory-leak-in-pgoutput-with-static-memory-co_PG17.patch
Type: text/x-patch
Part: 1
Message: Re: Memory leak in WAL sender with pgoutput (v10~)

Patch

Format: format-patch
Series: patch v1-0001
Subject: Fix memory leak in pgoutput with static memory context.
File+
src/backend/replication/pgoutput/pgoutput.c 11 6
From 3d2607acdeeb56715ddd29cdc9456470a457350f Mon Sep 17 00:00:00 2001
From: Vignesh <vignesh21@gmail.com>
Date: Tue, 10 Dec 2024 06:21:32 +0530
Subject: [PATCH v1] Fix memory leak in pgoutput with static memory context.

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 static memory context and
resets it each time the publication names cache is invalidated. This
ensures that the lifespan of the publication names aligns with that of
the logical decoding context.
---
 src/backend/replication/pgoutput/pgoutput.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index c9c952a278..b047d4dcd3 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2024,12 +2024,17 @@ 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;
-			}
+			static MemoryContext pubctx = NULL;
+
+			if (pubctx == NULL)
+				pubctx = AllocSetContextCreate(CacheMemoryContext,
+											   "logical replication publication list context",
+											   ALLOCSET_SMALL_SIZES);
+			else
+				MemoryContextReset(pubctx);
+
+			oldctx = MemoryContextSwitchTo(pubctx);
+
 			data->publications = LoadPublications(data->publication_names);
 			MemoryContextSwitchTo(oldctx);
 			publications_valid = true;
-- 
2.43.0