pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Fix memory leak in pgoutput with publication list cache
6+ messages / 1 participants
[nested] [flat]
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-09 07:43 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-09 07:43 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/f0c569d7151532fbc9c016b01af49f04a9fb5278
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 19 ++++++++++---------
src/include/replication/pgoutput.h | 1 +
2 files changed, 11 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-23 03:49 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-23 03:49 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
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
Branch
------
REL_15_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/6c9b3975407dcfaabd195c2e239cdd0451f7fbd1
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 46 +++++++++++++++++++++++------
1 file changed, 37 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-23 03:49 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-23 03:49 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
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
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/ba230ce40faa60463f4d0a57f5f2ececfd9e0974
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 41 +++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 5 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-23 03:49 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-23 03:49 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
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
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/cfd6cbcf9be078fbdf9587014231a5772039b386
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 41 +++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 5 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-23 03:49 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-23 03:49 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
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
Branch
------
REL_17_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/bbe68c13abe0f622cb29322651b9a0b67789358f
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 46 +++++++++++++++++++++++------
1 file changed, 37 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leak in pgoutput with publication list cache
@ 2024-12-23 03:49 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-12-23 03:49 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
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
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/4d45e7490c417d477458865926555895db125910
Modified Files
--------------
src/backend/replication/pgoutput/pgoutput.c | 46 +++++++++++++++++++++++------
1 file changed, 37 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-12-23 03:49 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-12-09 07:43 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
2024-12-23 03:49 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
2024-12-23 03:49 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
2024-12-23 03:49 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
2024-12-23 03:49 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
2024-12-23 03:49 pgsql: Fix memory leak in pgoutput with publication list cache Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox