PG-2229 Memory leaks once query executed via SPI interface#634
Open
artemgavrilov wants to merge 2 commits intomainfrom
Open
PG-2229 Memory leaks once query executed via SPI interface#634artemgavrilov wants to merge 2 commits intomainfrom
artemgavrilov wants to merge 2 commits intomainfrom
Conversation
artemgavrilov
changed the title
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
from
59caea0 to
1fd47f9
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #634 +/- ##
==========================================
+ Coverage 85.89% 86.08% +0.18%
==========================================
Files 3 3
Lines 1354 1358 +4
Branches 218 219 +1
==========================================
+ Hits 1163 1169 +6
+ Misses 91 90 -1
+ Partials 100 99 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
5 times, most recently
from
189046f to
1f47fbc
Compare
artemgavrilov
commented
Mar 20, 2026
| if (nesting_level == 0) | ||
| #else | ||
| if ((nesting_level + plan_nested_level) == 0) | ||
| #endif |
Contributor
Author
There was a problem hiding this comment.
Here we have to use try/finally because our context created under TopMemoryContext, so it lives forever. In this hook we may have an exception and we want to cleanup memory in this case too.
artemgavrilov
changed the title
Contributor
Author
|
BTW probably safer option will be to create our memory context under |
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
from
1f47fbc to
bd420f9
Compare
Contributor
Author
|
Something went wrong after sync with main branch, I'm on it. |
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
3 times, most recently
from
a7f6a00 to
dab4d33
Compare
artemgavrilov
commented
Mar 25, 2026
Comment on lines
+1029
to
+1033
| #if PG_VERSION_NUM < 160000 | ||
| PG_TRY(); | ||
| #else | ||
| PG_TRY(1); | ||
| #endif |
Contributor
Author
There was a problem hiding this comment.
Yes, this is ugly.
artemgavrilov
commented
Mar 25, 2026
Comment on lines
+1047
to
+1057
| nesting_level--; | ||
| } | ||
| #if PG_VERSION_NUM < 160000 | ||
| PG_CATCH(); | ||
| #else | ||
| PG_CATCH(1); | ||
| #endif | ||
| { | ||
| nesting_level--; | ||
| PG_RE_THROW(); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Actually this can be replaced with PG_FINALLY since we dropped PG12 support. Will do it in the following PG.
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
from
dab4d33 to
b089d3e
Compare
Attaching memory reset callback to MessageContext is wrong as for background workers that use SPI for query execution such context doesn't exist. There is no other context which lifetime matches our needs, so instead reset our memory context explicitly when we no longer need its data.
As we delete entry from the list we should also free its memory, even if we later reset whole memory context.
artemgavrilov
force-pushed
the
PG-2229-spi-memory-leak
branch
from
b089d3e to
34aa683
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PG-2229
Description
Before attached our memory context cleanup callback to
MessageContext, however this context doesn't exist when query executed via SPI. That caused memory leak. In this PR we explicitly reset our context when we no longer need its data, as there is no other context that matches our lifetime requirements.How to test
I used
worker_spiextension frompostgres/src/test/modules/worker_spiCREATE EXTENSION worker_spi;SELECT worker_spi_launch(1) IS NOT NULL;SELECT pid, query FROM pg_stat_activity;find PID of the process that executes queryWITH deleted AS (DELETE FROM...SELECT pg_log_backend_memory_contexts(<PID>);LOG: level: 2; pg_stat_monitor local store: 8192 total in 1 blocks; 7952 free (0 chunks); 240 used.Without this patch used space will constantly grow over time, while with the patch it will stay constant.
Links