Avoid longer-lived memory context for auto explain #7070
Comments
tedyu
changed the title
tedyu
added a commit
that referenced
this issue
Feb 3, 2021
Summary:
Upstream commit was 5c0f7cc5442108e113d4fb88c952329b467e2c6a
Commit message was:
```
The ExecutorEnd hook is invoked in a context that could be quite
long-lived, not the executor's own per-query context as I think
we were sort of assuming. Thus, any cruft generated while producing
the EXPLAIN output could accumulate over multiple queries. This can
result in spectacular leakage if log_nested_statements is on, and
even without that I'm surprised nobody complained before.
To fix, just switch into the executor's context so that anything we
allocate will be released when standard_ExecutorEnd frees the executor
state. We might as well nuke the code's retail pfree of the explain
output string, too; that's laughably inadequate to the need.
Japin Li, per report from Jeff Janes. This bug is old, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com
```
Test Plan: Build Yugabyte DB and run test suite via Jenkins
Reviewers: jason
Reviewed By: jason
Subscribers: yql
Differential Revision: https://phabricator.dev.yugabyte.com/D10529
polarweasel
pushed a commit
to lizayugabyte/yugabyte-db
that referenced
this issue
Mar 9, 2021
…_explain.'
Summary:
Upstream commit was 5c0f7cc5442108e113d4fb88c952329b467e2c6a
Commit message was:
```
The ExecutorEnd hook is invoked in a context that could be quite
long-lived, not the executor's own per-query context as I think
we were sort of assuming. Thus, any cruft generated while producing
the EXPLAIN output could accumulate over multiple queries. This can
result in spectacular leakage if log_nested_statements is on, and
even without that I'm surprised nobody complained before.
To fix, just switch into the executor's context so that anything we
allocate will be released when standard_ExecutorEnd frees the executor
state. We might as well nuke the code's retail pfree of the explain
output string, too; that's laughably inadequate to the need.
Japin Li, per report from Jeff Janes. This bug is old, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com
```
Test Plan: Build Yugabyte DB and run test suite via Jenkins
Reviewers: jason
Reviewed By: jason
Subscribers: yql
Differential Revision: https://phabricator.dev.yugabyte.com/D10529
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ExplainState is created in explain_ExecutorEnd() on SQL function
memory context, which is long-lived, causing the memory grow up.
queryDesc->estate->es_query_cxt memory context can be used instead.
The text was updated successfully, but these errors were encountered: