Re: [HACKERS] possible memory leak with SRFs
Nikhil Sontakke nikhil.sonta...@enterprisedb.com writes: Ok thanks. So if someone uses a really long-running srf with argument expression evaluations thrown in, then running into out of memory issues should be expected and then in those cases they are better off using multiple srf calls to get the same effect if they can.. I believe this is only an issue for SRFs called in a query targetlist, which is a usage fraught with semantic problems anyway. Hopefully we can get LATERAL done soon (I'm planning to look at it for 9.1) and then deprecate this whole mess. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
On 05/07/2010 09:06 PM, Nikhil Sontakke wrote: Yeah this is my basic confusion. But wouldn't the arguments be evaluated afresh on the subsequent call for this SRF? No, see ExecMakeFunctionResult(). If we did that we'd have serious problems with volatile functions, ie srf(random()). Ok thanks. So if someone uses a really long-running srf with argument expression evaluations thrown in, then running into out of memory issues should be expected and then in those cases they are better off using multiple srf calls to get the same effect if they can.. I've very recently looked into this exact case myself for someone, and came to the conclusion that there is no simple fix for this. If you want to see a concrete example of a query that fails, apply your patch and then run the regression tests -- the misc test will fail. I think this is an example of why we still need to implement a real SFRM_ValuePerCall mode that allows results to be pipelined. Yes, ValuePerCall sort of works from the targetlist, but it is pretty much useless for the use cases where people really want to use it. Or would a FROM clause ValuePerCall suffer the same issue? Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] possible memory leak with SRFs
Joe Conway m...@joeconway.com writes: I think this is an example of why we still need to implement a real SFRM_ValuePerCall mode that allows results to be pipelined. Yes, ValuePerCall sort of works from the targetlist, but it is pretty much useless for the use cases where people really want to use it. Or would a FROM clause ValuePerCall suffer the same issue? I don't think it'd be a big problem. We could use the technique suggested in the comments in ExecMakeTableFunctionResult: use a separate memory context for evaluating the arguments than for evaluating the function itself. This will work in FROM because we can insist the SRF be at top level. The problem with SRFs in tlists is that they can be anywhere and there can be more than one, so it's too hard to keep track of what to reset when. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
On 05/08/2010 09:12 AM, Tom Lane wrote: Joe Conway m...@joeconway.com writes: I think this is an example of why we still need to implement a real SFRM_ValuePerCall mode that allows results to be pipelined. Yes, ValuePerCall sort of works from the targetlist, but it is pretty much useless for the use cases where people really want to use it. Or would a FROM clause ValuePerCall suffer the same issue? I don't think it'd be a big problem. We could use the technique suggested in the comments in ExecMakeTableFunctionResult: use a separate memory context for evaluating the arguments than for evaluating the function itself. This will work in FROM because we can insist the SRF be at top level. The problem with SRFs in tlists is that they can be anywhere and there can be more than one, so it's too hard to keep track of what to reset when. That's what I was thinking. I saw your other email about LATERAL for 9.1 -- would it be helpful for me to work on this issue for 9.1? After all, about 7 years ago I said I'd do it ;-). Or do you think it will be an integral part of the LATERAL work? Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] possible memory leak with SRFs
Joe Conway m...@joeconway.com writes: That's what I was thinking. I saw your other email about LATERAL for 9.1 -- would it be helpful for me to work on this issue for 9.1? After all, about 7 years ago I said I'd do it ;-). Or do you think it will be an integral part of the LATERAL work? Dunno. What I was actually wanting to focus on is the problem of generalizing nestloop inner indexscans so that they can use parameters coming from more than one join level up. Robert suggested that LATERAL might fall out of that fairly easily, but I haven't thought much about it yet. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Hi, Can someone please explain why we do not reset the expression context if an SRF is involved during execution? Consider srf(foo(col)) where foo returns a pass-by-reference datatype. Your proposed patch would cut the knees out from under argument values that the SRF could reasonably expect to still be there on subsequent calls. Yeah this is my basic confusion. But wouldn't the arguments be evaluated afresh on the subsequent call for this SRF? In that case freeing up the context of the *last* call should not be an issue I would think. And if this is indeed the case we should be using a different longer lived context and not the ecxt_per_tuple_memory context.. Regards, Nikhils -- http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Nikhil Sontakke nikhil.sonta...@enterprisedb.com writes: Consider srf(foo(col)) where foo returns a pass-by-reference datatype. Yeah this is my basic confusion. But wouldn't the arguments be evaluated afresh on the subsequent call for this SRF? No, see ExecMakeFunctionResult(). If we did that we'd have serious problems with volatile functions, ie srf(random()). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Yeah this is my basic confusion. But wouldn't the arguments be evaluated afresh on the subsequent call for this SRF? No, see ExecMakeFunctionResult(). If we did that we'd have serious problems with volatile functions, ie srf(random()). Ok thanks. So if someone uses a really long-running srf with argument expression evaluations thrown in, then running into out of memory issues should be expected and then in those cases they are better off using multiple srf calls to get the same effect if they can.. Regards, Nikhils -- http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Hi,
Continuing on this:
Can someone please explain why we do not reset the expression context
if an SRF is involved during execution? Once the current result from
the SRF has been consumed, I would think that the
ecxt_per_tuple_memory context should be reset. As its name suggests,
it is supposed to a per tuple context and is not meant to be
long-lived. To test this out I shifted the call to ResetExprContext to
just before returning from the SRF inside ExecResult and I do not see
the memleak at all. Patch attached with this mail.
The SRF has its own long-lived SRF multi-call context anyways. And
AIUI, SRFs return tuples one-by-one or do we materialize the same into
a tuplestore in some cases?
Regards,
Nikhils
On Wed, May 5, 2010 at 7:23 PM, Nikhil Sontakke
nikhil.sonta...@enterprisedb.com wrote:
Hi,
I saw this behavior with latest GIT head:
create table xlarge(val numeric(19,0));
insert into xlarge values(generate_series(1,5));
The above generate series will return an int8 which will then be
casted to numeric (via int8_to_numericvar) before being inserted into
the table. I observed that the ExprContext memory associated with
econtext-ecxt_per_tuple_memory is slowly bloating up till the end of
the insert operation.
This becomes significant the moment we try to insert a significant
number of entries using this SRF. I can see the memory being consumed
by the PG backend slowly grow to a large percentage.
I see that the executor (take ExecResult as an example) does not reset
the expression context early if an SRF is churning out tuples. What
could be a good way to fix this?
Regards,
Nikhils
--
http://www.enterprisedb.com
--
http://www.enterprisedb.com
diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c
index 94796d5..bddc632 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -100,7 +100,10 @@ ExecResult(ResultState *node)
{
resultSlot = ExecProject(node-ps.ps_ProjInfo, isDone);
if (isDone == ExprMultipleResult)
+ {
+ ResetExprContext(econtext);
return resultSlot;
+ }
/* Done with that source tuple... */
node-ps.ps_TupFromTlist = false;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Patch attached with this mail. The previous patch was WIP. Please take a look at this one instead. I hope this handles the cases where results are not just base datums but palloc'ed datums like string types too. Regards, Nikhils The SRF has its own long-lived SRF multi-call context anyways. And AIUI, SRFs return tuples one-by-one or do we materialize the same into a tuplestore in some cases? Regards, Nikhils On Wed, May 5, 2010 at 7:23 PM, Nikhil Sontakke nikhil.sonta...@enterprisedb.com wrote: Hi, I saw this behavior with latest GIT head: create table xlarge(val numeric(19,0)); insert into xlarge values(generate_series(1,5)); The above generate series will return an int8 which will then be casted to numeric (via int8_to_numericvar) before being inserted into the table. I observed that the ExprContext memory associated with econtext-ecxt_per_tuple_memory is slowly bloating up till the end of the insert operation. This becomes significant the moment we try to insert a significant number of entries using this SRF. I can see the memory being consumed by the PG backend slowly grow to a large percentage. I see that the executor (take ExecResult as an example) does not reset the expression context early if an SRF is churning out tuples. What could be a good way to fix this? Regards, Nikhils -- http://www.enterprisedb.com -- http://www.enterprisedb.com -- http://www.enterprisedb.com diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c index 94796d5..ed437a0 100644 --- a/src/backend/executor/nodeResult.c +++ b/src/backend/executor/nodeResult.c @@ -91,6 +91,9 @@ ExecResult(ResultState *node) } } + /* release memory associated with the previous tuple */ + ResetExprContext(econtext); + /* * Check to see if we're still projecting out tuples from a previous scan * tuple (because there is a function-returning-set in the projection @@ -106,13 +109,6 @@ ExecResult(ResultState *node) } /* - * Reset per-tuple memory context to free any expression evaluation - * storage allocated in the previous tuple cycle. Note this can't happen - * until we're done projecting out tuples from a scan tuple. - */ - ResetExprContext(econtext); - - /* * if rs_done is true then it means that we were asked to return a * constant tuple and we already did the last time ExecResult() was * called, OR that we failed the constant qual check. Either way, now we -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible memory leak with SRFs
Nikhil Sontakke nikhil.sonta...@enterprisedb.com writes: Can someone please explain why we do not reset the expression context if an SRF is involved during execution? Consider srf(foo(col)) where foo returns a pass-by-reference datatype. Your proposed patch would cut the knees out from under argument values that the SRF could reasonably expect to still be there on subsequent calls. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] possible memory leak with SRFs
Hi, I saw this behavior with latest GIT head: create table xlarge(val numeric(19,0)); insert into xlarge values(generate_series(1,5)); The above generate series will return an int8 which will then be casted to numeric (via int8_to_numericvar) before being inserted into the table. I observed that the ExprContext memory associated with econtext-ecxt_per_tuple_memory is slowly bloating up till the end of the insert operation. This becomes significant the moment we try to insert a significant number of entries using this SRF. I can see the memory being consumed by the PG backend slowly grow to a large percentage. I see that the executor (take ExecResult as an example) does not reset the expression context early if an SRF is churning out tuples. What could be a good way to fix this? Regards, Nikhils -- http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
