Skip site navigation (1) Skip section navigation (2)

Re: Patch for memory leaks in index scan

From: Dmitry Tkach <dmitry(at)openratings(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)postgresql(dot)org, pgsql-patches(at)postgresql(dot)org
Subject: Re: Patch for memory leaks in index scan
Date: 2002-04-19 20:24:00
Message-ID: 3CC07CE0.1040901@openratings.com (view raw or flat)
Thread:
Lists: pgsql-bugspgsql-patches
Tom Lane wrote:

>Dmitry Tkach <dmitry(at)openratings(dot)com> writes:
>
>>*** nodeIndexscan.c.orig        Fri Apr 19 10:29:57 2002
>>--- nodeIndexscan.c     Fri Apr 19 10:30:00 2002
>>***************
>>*** 505,510 ****
>>--- 505,514 ----
>>          */
>>         ExecClearTuple(scanstate->cstate.cs_ResultTupleSlot);
>>         ExecClearTuple(scanstate->css_ScanTupleSlot);
>>+       pfree(scanstate);
>>+       pfree(indexstate->iss_RelationDescs);
>>+       pfree(indexstate->iss_ScanDescs);
>>+       pfree(indexstate);
>>   }
>>
>
>I do not believe this patch will fix anything.
>
Well... It does fix the query I mentioned in my first message (and a few 
others I was doing when I ran into this problem)

>
>In general, the EndNode routines do not bother with releasing memory,
>
Well... Not quite true. For example, ExecEndIndexScan () does release 
lots of stuff, that was allocated in ExecInitIndexScan (), it just 
forgets about these four things...

>
>because it's the end of the query and we're about to drop or reset
>the per-query context anyway. 
>
... if the query manages to complete without running out of memory like 
in my case :-)

> If the above pfrees are actually needed
>then there are a heck of a lot of other places that need explicit
>pfrees.  
>
Perhaps... I haven't run into any other places just yet...

>And that is not a path to a solution, because there will
>*always* be places that forgot a pfree. 
>
Sure :-)
You can say this about any bug pretty much :-) There will *always* be 
bugs... That doesn't mean that the ones, that are found should not be 
fixed though

> What's needed is a structural
>solution.
>
I understand what you are saying... I was looking around for one for a 
while, but it seems, but there doesn't seem
to be anything 'more structural' for this particular case, as far as I 
can see - per query context does get released properly in the end, it's 
just that the query requires too much temporary memory to complete, so 
the end is actually never reached... After all, I repeat, 
ExecEndIndexScan () DOES free up lots of memory, so I don't see how 
adding these four things that got forgotten would hurt.

>
>I think your real complaint is that SQL functions leak memory.  They
>should be creating a sub-context for their queries that can be freed
>when the function finishes.
>

I guess, you are right here - I tried using a subquery instead of a 
function, and that is not leaking, because it does
ExecRescan () for each outer tuple, instead of reinitializing the 
executor completely as it is the case with sql func .

fmgr_sql () DOES use it's own context, but it looks like it doesn't want 
to reset it every time, because of caching...

Perhaps, it could just execute every command in a function as a SubPlan, 
instead of reinitializing every time, but that wouldn't be a simple 4 
line fix, that certainly doesn't break anything that was working before :-)

I was thinking in terms of a quick PATCH, that can fix a particular 
problem, and would be safe to be put in.

It does not prevent any "structural solution" from being implemented if 
somebody comes up with one in the future, and it would STILL be useful 
even when then solution is implemented, because it  makes memory usage 
more conservative, thus reducing the load on system resources...

Frankly, I don't see what is your problem with it at all :-)

Dima





In response to

Responses

pgsql-bugs by date

Next:From: pgsql-bugsDate: 2002-04-20 09:04:18
Subject: Bug #637: pltcl bug with multibyte support
Previous:From: Tom LaneDate: 2002-04-19 19:37:59
Subject: Re: Patch for memory leaks in index scan

pgsql-patches by date

Next:From: Joe ConwayDate: 2002-04-19 20:29:33
Subject: Re: Odd(?) RI-trigger behavior
Previous:From: Tom LaneDate: 2002-04-19 19:37:59
Subject: Re: Patch for memory leaks in index scan

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group