From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: pgsql-bugs@lists.postgresql.org
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
Date: Sat, 11 Aug 2018 04:00:04 +0100
Message-ID: <87r2j57hdo.fsf@news-spur.riddles.org.uk> (raw)
In-Reply-To: <87va8h7nyl.fsf@news-spur.riddles.org.uk>
References: <153394403528.10284.7530399040974170549@wrigleys.postgresql.org>
<87va8h7nyl.fsf@news-spur.riddles.org.uk>
>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>>> * I removed the "buildercxt" memory context. It seemed mostly
>>> pointless, and I was disturbed by the MemoryContextResetOnly().
>>> Per-value memory still uses the per-value memory context, but the
>>> rest of the stuff is in the per-query context, which should be
>>> pretty much the same.
Andrew> A quick reading suggests that the per-value context should have
Andrew> been changed to not be a child of "buildercxt" (which would
Andrew> avoid the MemoryContextResetOnly issue - that's a good sign
Andrew> that you've put a child context under the wrong parent). But
Andrew> the use of the per-query context instead is exactly what causes
Andrew> this blowup; compare with what nodeFunctionscan does with its
Andrew> "argcontext" (and the corresponding comments in
Andrew> ExecMakeTableFunctionResult).
And here's a patch (against pg10, but I don't think the code's changed
since) that I think does it the right way.
This works by changing the meaning of perValueCxt - it's now the
per-INPUT-value context (equivalent to FunctionScan's argcontext, but I
didn't change the name for simplicity), not per-output-value; for the
latter, we use the result exprcontext's per-tuple memory like everything
else does (cf. FunctionScan).
I've verified that this fixes the leak; it also passes all other tests I
have thrown at it. Anyone see any issues with this? (CC'ing in Pavel as
original author)
I'll apply it in due course unless anyone says otherwise.
--
Andrew (irc:RhodiumToad)
Attachments:
[text/x-patch] xmlmem.patch (1.7K, ../87r2j57hdo.fsf@news-spur.riddles.org.uk/2-xmlmem.patch)
download | inline diff:diff --git a/src/backend/executor/nodeTableFuncscan.c b/src/backend/executor/nodeTableFuncscan.cindex b03d2ef762..222711bb5a 100644--- a/src/backend/executor/nodeTableFuncscan.c+++ b/src/backend/executor/nodeTableFuncscan.c@@ -288,6 +288,9 @@ tfuncFetchRows(TableFuncScanState *tstate, ExprContext *econtext)
oldcxt = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
tstate->tupstore = tuplestore_begin_heap(false, false, work_mem);
+ MemoryContextReset(tstate->perValueCxt);+ MemoryContextSwitchTo(tstate->perValueCxt);+
PG_TRY();
{
routine->InitOpaque(tstate,
@@ -319,8 +322,7 @@ tfuncFetchRows(TableFuncScanState *tstate, ExprContext *econtext)
}
PG_END_TRY();
- /* return to original memory context, and clean up */- MemoryContextSwitchTo(oldcxt);+ /* clean up and return to original memory context */
if (tstate->opaque != NULL)
{
@@ -328,6 +330,9 @@ tfuncFetchRows(TableFuncScanState *tstate, ExprContext *econtext)
tstate->opaque = NULL;
}
+ MemoryContextSwitchTo(oldcxt);+ MemoryContextReset(tstate->perValueCxt);+
return;
}
@@ -433,7 +438,7 @@ tfuncLoadRows(TableFuncScanState *tstate, ExprContext *econtext)
ordinalitycol =
((TableFuncScan *) (tstate->ss.ps.plan))->tablefunc->ordinalitycol;
- oldcxt = MemoryContextSwitchTo(tstate->perValueCxt);+ oldcxt = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
/*
* Keep requesting rows from the table builder until there aren't any.
@@ -496,7 +501,7 @@ tfuncLoadRows(TableFuncScanState *tstate, ExprContext *econtext)
tuplestore_putvalues(tstate->tupstore, tupdesc, values, nulls);
- MemoryContextReset(tstate->perValueCxt);+ MemoryContextReset(econtext->ecxt_per_tuple_memory);
}
MemoryContextSwitchTo(oldcxt);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: andrew@tao11.riddles.org.uk, pgsql-bugs@lists.postgresql.org, alvherre@2ndquadrant.com, pavel.stehule@gmail.com
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
In-Reply-To: <87r2j57hdo.fsf@news-spur.riddles.org.uk>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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