Horton

Thread

Commits

  1. Fix transient memory leak for SRFs in FROM.

  2. Fix two memory leaks around force-storing tuples in slots.

  3. Rejigger materializing and fetching a HeapTuple from a slot.

  1. BUG #16112: large, unexpected memory consumption

    The Post Office <noreply@postgresql.org> — 2019-11-13T12:22:07Z

    The following bug has been logged on the website:

Bug reference:      16112
Logged by:          Ben Cornett
Email address:      ben@lantern.is
PostgreSQL version: 12.0
Operating system:   linux 2.6.32
Description:        

Creation of table t1 in the query below causes the server process to consume
close to 1GB of memory.  The amount of memory consumed is proportional to
the value passed to generate_series in the first query.

CREATE temp TABLE t0 AS
SELECT
        i,
        ARRAY[1,2,3] a
FROM GENERATE_SERIES(1, 12000000) i
;

CREATE TEMP TABLE t1 AS
SELECT
        i,
        x
FROM t0, UNNEST(a) x;

I observed the same behavior in other test queries that included implicit
lateral joins.

Thanks very much,
Ben

  2. Re: BUG #16112: large, unexpected memory consumption

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-11-13T14:50:04Z

    On Wed, Nov 13, 2019 at 12:22:07PM +0000, PG Bug reporting form wrote:
>The following bug has been logged on the website:
>
>Bug reference:      16112
>Logged by:          Ben Cornett
>Email address:      ben@lantern.is
>PostgreSQL version: 12.0
>Operating system:   linux 2.6.32
>Description:
>
>Creation of table t1 in the query below causes the server process to consume
>close to 1GB of memory.  The amount of memory consumed is proportional to
>the value passed to generate_series in the first query.
>
>CREATE temp TABLE t0 AS
>SELECT
>        i,
>        ARRAY[1,2,3] a
>FROM GENERATE_SERIES(1, 12000000) i
>;
>
>CREATE TEMP TABLE t1 AS
>SELECT
>        i,
>        x
>FROM t0, UNNEST(a) x;
>
>I observed the same behavior in other test queries that included implicit
>lateral joins.
>

Yeah, I can reproduce this pretty easily. It seems like a memory leak in
ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
variable-length, but it gets allocated in ExecutorState context directly
and so until the end of the query.

The attached trivial patch fixes that by adding a pfree() at the end of
the function. I wonder if we have the same issue elsewhere ...


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2019-11-13T17:05:28Z

    Hi,

On 2019-11-13 15:50:04 +0100, Tomas Vondra wrote:
> Yeah, I can reproduce this pretty easily. It seems like a memory leak in
> ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
> variable-length, but it gets allocated in ExecutorState context directly
> and so until the end of the query.

Damn. Too bad this got discovered just after the point release was
wrapped :(


> The attached trivial patch fixes that by adding a pfree() at the end of
> the function.

Hm. That's clearly an improvement. But I'm not quite sure it's really
the right direction. It seems like a bad idea to rely on
ExecMakeTableFunctionResult() otherwise never leaking any memory.

It seems to we should be using memory contexts to provide a backstop
against leaks, like we normally do, instead of operating in the
per-query context. It's noteworthy that nodeProjectSet already does so.
In contrast to nodeProjectSet, I think we'd need an additional memory
context however, as we eagerly evaluate ValuePerCall expressions - and
we'd like to reset the context after each iteration.


I think we also ought to use SetExprState->fcinfo, instead of allocating
a separate allocation in ExecMakeTableFunctionResult(). But that's
perhaps less important.


> I wonder if we have the same issue elsewhere ...

Quite possible - but I think in most cases we are using memory contexts
to protect against leaks like this (which is more efficient than retail
pfree'ing).

Greetings,

Andres Freund

  4. Re: BUG #16112: large, unexpected memory consumption

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-13T17:21:47Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-11-13 15:50:04 +0100, Tomas Vondra wrote:
>> The attached trivial patch fixes that by adding a pfree() at the end of
>> the function.

> Hm. That's clearly an improvement. But I'm not quite sure it's really
> the right direction. It seems like a bad idea to rely on
> ExecMakeTableFunctionResult() otherwise never leaking any memory.

Considering that ExecMakeTableFunctionResult went from zero pallocs
to one, I don't see a strong reason why it should have bothered with
a private memory context before, nor do I think that's a good
response now.

			regards, tom lane

  5. Re: BUG #16112: large, unexpected memory consumption

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-11-13T17:28:03Z

    On Wed, Nov 13, 2019 at 09:05:28AM -0800, Andres Freund wrote:
>Hi,
>
>On 2019-11-13 15:50:04 +0100, Tomas Vondra wrote:
>> Yeah, I can reproduce this pretty easily. It seems like a memory leak in
>> ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
>> variable-length, but it gets allocated in ExecutorState context directly
>> and so until the end of the query.
>
>Damn. Too bad this got discovered just after the point release was
>wrapped :(
>
>
>> The attached trivial patch fixes that by adding a pfree() at the end of
>> the function.
>
>Hm. That's clearly an improvement. But I'm not quite sure it's really
>the right direction. It seems like a bad idea to rely on
>ExecMakeTableFunctionResult() otherwise never leaking any memory.
>
>It seems to we should be using memory contexts to provide a backstop
>against leaks, like we normally do, instead of operating in the
>per-query context. It's noteworthy that nodeProjectSet already does so.
>In contrast to nodeProjectSet, I think we'd need an additional memory
>context however, as we eagerly evaluate ValuePerCall expressions - and
>we'd like to reset the context after each iteration.
>

Possibly, but I think most of the allocations already use the per-tuple
context. It's just the fcinfo that seems to be allocated outside of it,
so maybe we should just move it to that memory context.

>
>I think we also ought to use SetExprState->fcinfo, instead of allocating
>a separate allocation in ExecMakeTableFunctionResult(). But that's
>perhaps less important.
>

Maybe.

>
>> I wonder if we have the same issue elsewhere ...
>
>Quite possible - but I think in most cases we are using memory contexts
>to protect against leaks like this (which is more efficient than retail
>pfree'ing).
>

Yeah, probably. I had a quick look at some of those places (found by
looking for SizeForFunctionCallInfo and palloc on the same line) and
those that I looked at seemed fine.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2019-11-13T17:47:33Z

    Hi,

On 2019-11-13 12:21:47 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-11-13 15:50:04 +0100, Tomas Vondra wrote:
> >> The attached trivial patch fixes that by adding a pfree() at the end of
> >> the function.
> 
> > Hm. That's clearly an improvement. But I'm not quite sure it's really
> > the right direction. It seems like a bad idea to rely on
> > ExecMakeTableFunctionResult() otherwise never leaking any memory.
> 
> Considering that ExecMakeTableFunctionResult went from zero pallocs
> to one, I don't see a strong reason why it should have bothered with
> a private memory context before, nor do I think that's a good
> response now.

Well, that relies on type_is_rowtype(), exprType(),
TupleDescInitEntry(), lookup_rowtype_tupdesc_copy() not to leak memory
(besides lookup_rowtype_tupdesc_copy()'s return type, which is
explicitly freed). Which is true, but still seems not super reliable.

Thinking about it for a second longer, I don't think we'd need a new
context - afaict argcontext has exactly the lifetime requirements
needed.

Greetings,

Andres Freund

  7. Re: BUG #16112: large, unexpected memory consumption

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-13T18:03:31Z

    Andres Freund <andres@anarazel.de> writes:
> Thinking about it for a second longer, I don't think we'd need a new
> context - afaict argcontext has exactly the lifetime requirements
> needed.

Hm, maybe so.  That'd definitely be a better solution if it works.

			regards, tom lane

  8. Re: BUG #16112: large, unexpected memory consumption

    Jeff Janes <jeff.janes@gmail.com> — 2019-11-13T22:53:28Z

    On Wed, Nov 13, 2019 at 9:50 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:

>
> Yeah, I can reproduce this pretty easily. It seems like a memory leak in
> ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
> variable-length, but it gets allocated in ExecutorState context directly
> and so until the end of the query.
>

I find the leak was introduced much earlier than that, in:

commit 763f2edd92095b1ca2f4476da073a28505c13820
Author: Andres Freund <andres@anarazel.de>
Date:   Thu Nov 15 14:26:14 2018 -0800

    Rejigger materializing and fetching a HeapTuple from a slot.

I have no idea if this info is useful to informing the best solution,
though.

You patch applied to REL_12_STABLE does fix it for me.


> The attached trivial patch fixes that by adding a pfree() at the end of
> the function. I wonder if we have the same issue elsewhere ...
>
>
Is there an easy way to assess if the "make check" regression tests are
taking more memory than they used to?

Cheers,

Jeff

  9. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2019-11-13T23:04:07Z

    Hi,

On 2019-11-13 17:53:28 -0500, Jeff Janes wrote:
> On Wed, Nov 13, 2019 at 9:50 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
> wrote:
> 
> >
> > Yeah, I can reproduce this pretty easily. It seems like a memory leak in
> > ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
> > variable-length, but it gets allocated in ExecutorState context directly
> > and so until the end of the query.
> >
> 
> I find the leak was introduced much earlier than that, in:
> 
> commit 763f2edd92095b1ca2f4476da073a28505c13820
> Author: Andres Freund <andres@anarazel.de>
> Date:   Thu Nov 15 14:26:14 2018 -0800
> 
>     Rejigger materializing and fetching a HeapTuple from a slot.
> 
> I have no idea if this info is useful to informing the best solution,
> though.
> 
> You patch applied to REL_12_STABLE does fix it for me.

I think that's likely two overlapping issues.

Possibly
commit 88e6ad3054ddd5aa0dee12e5def2c335fe92a414
Author: Andres Freund <andres@anarazel.de>
Date:   2019-04-19 11:33:37 -0700

    Fix two memory leaks around force-storing tuples in slots.



> > The attached trivial patch fixes that by adding a pfree() at the end of
> > the function. I wonder if we have the same issue elsewhere ...
> >
> >
> Is there an easy way to assess if the "make check" regression tests are
> taking more memory than they used to?

Not easily, I think. I also suspect that you'd not have seen a
meaningful increase in memory usage due to this bug - it "only" was
noticable for the query at hand, because the FunctionScan node was
reached so many times, due to the lateral join forcing it to be scanned
once for each value in the source temp table. I don't think we have a
query in the tests doing so often enough (partially because we don't,
partially because it'd take a lot of time on slow machines).

Greetings,

Andres Freund

  10. Re: BUG #16112: large, unexpected memory consumption

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-11-13T23:18:27Z

    On Wed, Nov 13, 2019 at 05:53:28PM -0500, Jeff Janes wrote:
>On Wed, Nov 13, 2019 at 9:50 AM Tomas Vondra <tomas.vondra@2ndquadrant.com>
>wrote:
>
>>
>> Yeah, I can reproduce this pretty easily. It seems like a memory leak in
>> ExecMakeTableFunctionResult. a9c35cf85ca reworked FunctionCallInfo to be
>> variable-length, but it gets allocated in ExecutorState context directly
>> and so until the end of the query.
>>
>
>I find the leak was introduced much earlier than that, in:
>
>commit 763f2edd92095b1ca2f4476da073a28505c13820
>Author: Andres Freund <andres@anarazel.de>
>Date:   Thu Nov 15 14:26:14 2018 -0800
>
>    Rejigger materializing and fetching a HeapTuple from a slot.
>
>I have no idea if this info is useful to informing the best solution,
>though.
>

Ah, I've only done a simple 'git blame' and assumed it's the last commit
that touched the palloc line (and it seemed somewhat plausible). I don't
think it changes the reasoning too much, though.

>You patch applied to REL_12_STABLE does fix it for me.
>
>
>> The attached trivial patch fixes that by adding a pfree() at the end of
>> the function. I wonder if we have the same issue elsewhere ...
>>
>>
>Is there an easy way to assess if the "make check" regression tests are
>taking more memory than they used to?
>

Probably not. The regression tests use fairly small number of rows in
general, and we don't have a way to track/inspect high watermaks or
anything like that anyway.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2019-11-15T02:47:07Z

    Hi,

On 2019-11-13 13:03:31 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Thinking about it for a second longer, I don't think we'd need a new
> > context - afaict argcontext has exactly the lifetime requirements
> > needed.
> 
> Hm, maybe so.  That'd definitely be a better solution if it works.

Here's a patch doing so. I think it'd be a good idea to rename
argcontext into something like setcontext or such. But as I'm not that
happy with the name, I've not yet made that change.

Greetings,

Andres Freund

  12. Re: BUG #16112: large, unexpected memory consumption

    Ben Cornett <ben@lantern.is> — 2020-04-09T12:21:54Z

    Hi,

Did this patch ever get applied?  It is not clear to me that it did.

Thanks!

On Thu, Nov 14, 2019 at 06:47:07PM -0800, Andres Freund wrote:
> Hi,
> 
> On 2019-11-13 13:03:31 -0500, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > Thinking about it for a second longer, I don't think we'd need a new
> > > context - afaict argcontext has exactly the lifetime requirements
> > > needed.
> > 
> > Hm, maybe so.  That'd definitely be a better solution if it works.
> 
> Here's a patch doing so. I think it'd be a good idea to rename
> argcontext into something like setcontext or such. But as I'm not that
> happy with the name, I've not yet made that change.
> 
> Greetings,
> 
> Andres Freund

> diff --git i/src/backend/executor/execSRF.c w/src/backend/executor/execSRF.c
> index c8a3efc3654..85f33054265 100644
> --- i/src/backend/executor/execSRF.c
> +++ w/src/backend/executor/execSRF.c
> @@ -114,10 +114,23 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>  	ReturnSetInfo rsinfo;
>  	HeapTupleData tmptup;
>  	MemoryContext callerContext;
> -	MemoryContext oldcontext;
>  	bool		first_time = true;
>  
> -	callerContext = CurrentMemoryContext;
> +	/*
> +	 * Execute per-tablefunc actions in appropriate context.
> +	 *
> +	 * The FunctionCallInfo needs to live across all the calls to a
> +	 * ValuePerCall function, so it can't be allocated in the per-tuple
> +	 * context. Similarly, the function arguments need to be evaluated in a
> +	 * context that is longer lived than the per-tuple context: The argument
> +	 * values would otherwise disappear when we reset that context in the
> +	 * inner loop.  As the caller's CurrentMemoryContext is typically a
> +	 * query-lifespan context, we don't want to leak memory there.  We require
> +	 * the caller to pass a separate memory context that can be used for this,
> +	 * and can be reset each time through to avoid bloat.
> +	 */
> +	MemoryContextReset(argContext);
> +	callerContext = MemoryContextSwitchTo(argContext);
>  
>  	funcrettype = exprType((Node *) setexpr->expr);
>  
> @@ -164,20 +177,9 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>  								 setexpr->fcinfo->fncollation,
>  								 NULL, (Node *) &rsinfo);
>  
> -		/*
> -		 * Evaluate the function's argument list.
> -		 *
> -		 * We can't do this in the per-tuple context: the argument values
> -		 * would disappear when we reset that context in the inner loop.  And
> -		 * the caller's CurrentMemoryContext is typically a query-lifespan
> -		 * context, so we don't want to leak memory there.  We require the
> -		 * caller to pass a separate memory context that can be used for this,
> -		 * and can be reset each time through to avoid bloat.
> -		 */
> -		MemoryContextReset(argContext);
> -		oldcontext = MemoryContextSwitchTo(argContext);
> +		/* evaluate the function's argument list */
> +		Assert(CurrentMemoryContext == argContext);
>  		ExecEvalFuncArgs(fcinfo, setexpr->args, econtext);
> -		MemoryContextSwitchTo(oldcontext);
>  
>  		/*
>  		 * If function is strict, and there are any NULL arguments, skip
> @@ -217,7 +219,7 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>  		CHECK_FOR_INTERRUPTS();
>  
>  		/*
> -		 * reset per-tuple memory context before each call of the function or
> +		 * Reset per-tuple memory context before each call of the function or
>  		 * expression. This cleans up any local memory the function may leak
>  		 * when called.
>  		 */
> @@ -257,6 +259,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>  			 */
>  			if (first_time)
>  			{
> +				MemoryContext oldcontext;
> +
>  				oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
>  				tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
>  				rsinfo.setResult = tupstore;
> @@ -285,6 +289,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>  
>  					if (tupdesc == NULL)
>  					{
> +						MemoryContext oldcontext;
> +
>  						/*
>  						 * This is the first non-NULL result from the
>  						 * function.  Use the type info embedded in the
> @@ -378,15 +384,18 @@ no_function_result:
>  	 */
>  	if (rsinfo.setResult == NULL)
>  	{
> -		MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
> +		MemoryContext oldcontext;
> +
> +		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
>  		tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
>  		rsinfo.setResult = tupstore;
> +		MemoryContextSwitchTo(oldcontext);
> +
>  		if (!returnsSet)
>  		{
>  			int			natts = expectedDesc->natts;
>  			bool	   *nullflags;
>  
> -			MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
>  			nullflags = (bool *) palloc(natts * sizeof(bool));
>  			memset(nullflags, true, natts * sizeof(bool));
>  			tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);

  13. Re: BUG #16112: large, unexpected memory consumption

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2020-04-11T18:35:52Z

    On Thu, Apr 09, 2020 at 08:21:54AM -0400, Ben Cornett wrote:
>Hi,
>
>Did this patch ever get applied?  It is not clear to me that it did.
>

Hmmm, I don't think it got applied, which means we've missed yet another
minor release :-(

Andres, any plans to push this?

regards

>Thanks!
>
>On Thu, Nov 14, 2019 at 06:47:07PM -0800, Andres Freund wrote:
>> Hi,
>>
>> On 2019-11-13 13:03:31 -0500, Tom Lane wrote:
>> > Andres Freund <andres@anarazel.de> writes:
>> > > Thinking about it for a second longer, I don't think we'd need a new
>> > > context - afaict argcontext has exactly the lifetime requirements
>> > > needed.
>> >
>> > Hm, maybe so.  That'd definitely be a better solution if it works.
>>
>> Here's a patch doing so. I think it'd be a good idea to rename
>> argcontext into something like setcontext or such. But as I'm not that
>> happy with the name, I've not yet made that change.
>>
>> Greetings,
>>
>> Andres Freund
>
>> diff --git i/src/backend/executor/execSRF.c w/src/backend/executor/execSRF.c
>> index c8a3efc3654..85f33054265 100644
>> --- i/src/backend/executor/execSRF.c
>> +++ w/src/backend/executor/execSRF.c
>> @@ -114,10 +114,23 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>>  	ReturnSetInfo rsinfo;
>>  	HeapTupleData tmptup;
>>  	MemoryContext callerContext;
>> -	MemoryContext oldcontext;
>>  	bool		first_time = true;
>>
>> -	callerContext = CurrentMemoryContext;
>> +	/*
>> +	 * Execute per-tablefunc actions in appropriate context.
>> +	 *
>> +	 * The FunctionCallInfo needs to live across all the calls to a
>> +	 * ValuePerCall function, so it can't be allocated in the per-tuple
>> +	 * context. Similarly, the function arguments need to be evaluated in a
>> +	 * context that is longer lived than the per-tuple context: The argument
>> +	 * values would otherwise disappear when we reset that context in the
>> +	 * inner loop.  As the caller's CurrentMemoryContext is typically a
>> +	 * query-lifespan context, we don't want to leak memory there.  We require
>> +	 * the caller to pass a separate memory context that can be used for this,
>> +	 * and can be reset each time through to avoid bloat.
>> +	 */
>> +	MemoryContextReset(argContext);
>> +	callerContext = MemoryContextSwitchTo(argContext);
>>
>>  	funcrettype = exprType((Node *) setexpr->expr);
>>
>> @@ -164,20 +177,9 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>>  								 setexpr->fcinfo->fncollation,
>>  								 NULL, (Node *) &rsinfo);
>>
>> -		/*
>> -		 * Evaluate the function's argument list.
>> -		 *
>> -		 * We can't do this in the per-tuple context: the argument values
>> -		 * would disappear when we reset that context in the inner loop.  And
>> -		 * the caller's CurrentMemoryContext is typically a query-lifespan
>> -		 * context, so we don't want to leak memory there.  We require the
>> -		 * caller to pass a separate memory context that can be used for this,
>> -		 * and can be reset each time through to avoid bloat.
>> -		 */
>> -		MemoryContextReset(argContext);
>> -		oldcontext = MemoryContextSwitchTo(argContext);
>> +		/* evaluate the function's argument list */
>> +		Assert(CurrentMemoryContext == argContext);
>>  		ExecEvalFuncArgs(fcinfo, setexpr->args, econtext);
>> -		MemoryContextSwitchTo(oldcontext);
>>
>>  		/*
>>  		 * If function is strict, and there are any NULL arguments, skip
>> @@ -217,7 +219,7 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>>  		CHECK_FOR_INTERRUPTS();
>>
>>  		/*
>> -		 * reset per-tuple memory context before each call of the function or
>> +		 * Reset per-tuple memory context before each call of the function or
>>  		 * expression. This cleans up any local memory the function may leak
>>  		 * when called.
>>  		 */
>> @@ -257,6 +259,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>>  			 */
>>  			if (first_time)
>>  			{
>> +				MemoryContext oldcontext;
>> +
>>  				oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
>>  				tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
>>  				rsinfo.setResult = tupstore;
>> @@ -285,6 +289,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr,
>>
>>  					if (tupdesc == NULL)
>>  					{
>> +						MemoryContext oldcontext;
>> +
>>  						/*
>>  						 * This is the first non-NULL result from the
>>  						 * function.  Use the type info embedded in the
>> @@ -378,15 +384,18 @@ no_function_result:
>>  	 */
>>  	if (rsinfo.setResult == NULL)
>>  	{
>> -		MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
>> +		MemoryContext oldcontext;
>> +
>> +		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
>>  		tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
>>  		rsinfo.setResult = tupstore;
>> +		MemoryContextSwitchTo(oldcontext);
>> +
>>  		if (!returnsSet)
>>  		{
>>  			int			natts = expectedDesc->natts;
>>  			bool	   *nullflags;
>>
>> -			MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
>>  			nullflags = (bool *) palloc(natts * sizeof(bool));
>>  			memset(nullflags, true, natts * sizeof(bool));
>>  			tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);
>
>
>

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  14. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2020-04-20T18:55:30Z

    Hi,

On 2020-04-11 20:35:52 +0200, Tomas Vondra wrote:
> On Thu, Apr 09, 2020 at 08:21:54AM -0400, Ben Cornett wrote:

Ben, for the future, it's better to not drop CC addresses etc. I didn't
see this reply at the time you sent it, just when I was catching up on
my backlog of unread postgres email.


> > Did this patch ever get applied?  It is not clear to me that it did.

No :(. I lost track of it. I did recall that there was some patch I was
supposed to get back to, but couldn't remember what exactly :(.

It's really too bad that we don't have a bugtracker. I just can't keep
track of all bugs / patches where I e.g. want to wait a few days for a
reply (here to see whether Tom likes my alternative context based
approach).


> Hmmm, I don't think it got applied, which means we've missed yet another
> minor release :-(
> 
> Andres, any plans to push this?

Yea, I think I'll go for my alternative approach from
https://postgr.es/m/20191115024707.y5bcsb4oo4wzovu4%40alap3.anarazel.de

Greetings,

Andres Freund

  15. Re: BUG #16112: large, unexpected memory consumption

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-04-20T19:19:13Z

    Andres Freund <andres@anarazel.de> writes:
> It's really too bad that we don't have a bugtracker. I just can't keep
> track of all bugs / patches where I e.g. want to wait a few days for a
> reply (here to see whether Tom likes my alternative context based
> approach).

> Yea, I think I'll go for my alternative approach from
> https://postgr.es/m/20191115024707.y5bcsb4oo4wzovu4%40alap3.anarazel.de

I haven't done any testing, but it looks reasonable.

			regards, tom lane

  16. Re: BUG #16112: large, unexpected memory consumption

    Andres Freund <andres@anarazel.de> — 2020-04-23T03:00:07Z

    Hi,

On 2020-04-20 15:19:13 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > It's really too bad that we don't have a bugtracker. I just can't keep
> > track of all bugs / patches where I e.g. want to wait a few days for a
> > reply (here to see whether Tom likes my alternative context based
> > approach).
> 
> > Yea, I think I'll go for my alternative approach from
> > https://postgr.es/m/20191115024707.y5bcsb4oo4wzovu4%40alap3.anarazel.de
> 
> I haven't done any testing, but it looks reasonable.

Thanks for looking. Pushed.

Ben, thanks for the report. And sorry that it took so long.

Greetings,

Andres Freund