Thread: Facing a problem with SPI
<span style="font-family: courier new,monospace;">Hi all,<br /><br /> I am facing a problem with the SPI (spi.c). I haveimplemented short-lived virtual indexes using SubTransactions. The Index Adviser, when creating virtual indexes, createsthem inside a SubTransaction, and when it is done with its advisory stuff, it rolls back the SubTransaction. Thisachieves two objectives: <br /><br /> (1) Easy cleanup of dependencies/changes in the catalog.<br /> (2) The virtualindexes are not visible to other backends. This eliminates the possibility of other backends (not running in Advisemode) seeing these indexes, as these don't have any data in them. <br /><br /> So all the index creation/deletionis done between a pair of BeginInternalSubTransaction() (BIST()) and RollbackAndReleaseCurrentSubTransaction()(RARCST()). This all works well when we are EXPLAINing query in special advise mode.<br /><br /> Now, based on this, I wanted to generate advise for every query that comes for planning, be it frompsql or pl/pgsql or anywhere else. So I made a function call to the index_adviser() from within pg_plan_query() afterthe planner() call. <br /><br /> The problem I am facing is that that the queries being planned by pl/pgsql, comeafter a call to SPI_connect() and SPI_Prepare(). SPI_prepare() switches to _SPI_current->execCxt memory context using_SPI_begin_call(). <br /><br /> Everything goes well until the index_adviser() calls RARCST() to rollback it's SubTransaction.RARCST() ultimately ends up calling AtEOSubXact_SPI(), which, just before returning, deletes the executionmemory context of the surrounding SPI (which happens to be pl/pgsql's SPI). This, in effect, frees all the memoryallocated by pl/pgsql for the prepare stage; and the next time the pl/pgsql's data-structures are referenced, it causesa crash. The code in question is: <br /><br />void<br />AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)<br/>{<br /> bool found = false;<br /><br /> while (_SPI_connected >= 0)<br /> {<br /> _SPI_connection *connection = &(_SPI_stack[_SPI_connected]); <br /><br /> if (connection->connectSubid!= mySubid)<br /> break; /* couldn't be any underneath it either */<br/><br /> ......<br /> ......<br /> /*<br /> * If we are aborting a subtransaction and there isan open SPI context <br /> * surrounding the subxact, clean up to prevent memory leakage.<br /> */<br /> if(_SPI_current && !isCommit)<br /> {<br /> /* free Executor memory the same as _SPI_end_call would do*/<br /> MemoryContextResetAndDeleteChildren(_SPI_current->execCxt); <br /> ^^^^^^^^^^^<br /> /*throw away any partially created tuple-table */<br /> SPI_freetuptable(_SPI_current->tuptable);<br /> _SPI_current->tuptable = NULL;<br /> }<br />}<br /><br /> The code is doing what the comments say, buthow I wish it didn't. This code assumes that the latest BIST() came from a SPI invoking module, which the Index Adviseris not! <br /><br /> I haven't been able to come up with a proof, but I think this situation can be simulated/reproducedusing some pl/* language that we support.<br /><br /> pl/pgsql is safe, since it doesn't allow SAVEPOINT/ROLLBACKTO SAVEPOINT inside it's code blocks; but if some pl/xxx allows SAVEPOINTS, I think it can be reproducedusing something like: <br /><br /> SAVEPOINT xxx ( should call BIST() )<br /> SAVEPOINT yyy ( should call BIST() )<br /> Rollback (to yyy) ( should call RARCST() ) will free pl/xxx exec context<br/> Rollback (to xxx) ( should call RARCST() ) <br /><br /> Probably just one pair, instead of twonested ones, should reproduce it. I tried to reproduce it using pl/pgsql's exception block (there we use BIST() and RARCST());but couldn't succeed.<br /><br /> I hope I have understood the issue correctly, or have I missed something?Is there a way to work around this? <br /><br />Best regards,<br clear="all" style="font-family: courier new,monospace;"/></span><br style="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">--</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">gurjeet[.singh]@EnterpriseDB.com</span><brstyle="font-family: courier new,monospace;" /><span style="font-family:courier new,monospace;">singh.gurjeet@{ gmail | hotmail | yahoo }.com </span>
<span style="font-family: courier new,monospace;"> Just noticed that this was committed by Tom on 21/Nov with this logmessage:</span><br style="font-family: courier new,monospace;" /><br style="font-family: courier new,monospace;" /><spanstyle="font-family: courier new,monospace;"><snip></span><br style="font-family: courier new,monospace;" /><spanstyle="font-family: courier new,monospace;">Prevent intratransaction memory leak when a subtransaction is aborted</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">in themiddle of executing a SPI query. This doesn't entirely fix the</span><br style="font-family: courier new,monospace;"/><span style="font-family: courier new,monospace;">problem of memory leakage in plpgsql exception handling,but it should</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">get rid of the lion's share of leakage.<br /></snip><br /><br />Can the memory leaks be avoided insome other way?<br /><br />Best regards,<br /><br />-- </span><br style="font-family: courier new,monospace;" /><span style="font-family:courier new,monospace;"> gurjeet[.singh]@EnterpriseDB.com</span><br style="font-family: courier new,monospace;"/><span style="font-family: courier new,monospace;">singh.gurjeet@{ gmail | hotmail | yahoo }.com </span><brstyle="font-family: courier new,monospace;" /><br style="font-family: courier new,monospace;" /><div style="font-family:courier new,monospace;"><span class="gmail_quote">On 12/3/06, <b class="gmail_sendername">Gurjeet Singh</b> <<a href="mailto:singh.gurjeet@gmail.com">singh.gurjeet@gmail.com</a>> wrote:</span><blockquote class="gmail_quote"style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Hi all,<br/><br /> I am facing a problem with the SPI (spi.c). I have implemented short-lived virtual indexes using SubTransactions.The Index Adviser, when creating virtual indexes, creates them inside a SubTransaction, and when it is donewith its advisory stuff, it rolls back the SubTransaction. This achieves two objectives: <br /><br /> (1) Easy cleanupof dependencies/changes in the catalog.<br /> (2) The virtual indexes are not visible to other backends. This eliminatesthe possibility of other backends (not running in Advise mode) seeing these indexes, as these don't have any datain them. <br /><br /> So all the index creation/deletion is done between a pair of BeginInternalSubTransaction() (BIST())and RollbackAndReleaseCurrentSubTransaction() (RARCST()). This all works well when we are EXPLAINing query in specialadvise mode. <br /><br /> Now, based on this, I wanted to generate advise for every query that comes for planning,be it from psql or pl/pgsql or anywhere else. So I made a function call to the index_adviser() from within pg_plan_query()after the planner() call. <br /><br /> The problem I am facing is that that the queries being planned bypl/pgsql, come after a call to SPI_connect() and SPI_Prepare(). SPI_prepare() switches to _SPI_current->execCxt memorycontext using _SPI_begin_call(). <br /><br /> Everything goes well until the index_adviser() calls RARCST() to rollbackit's SubTransaction. RARCST() ultimately ends up calling AtEOSubXact_SPI(), which, just before returning, deletesthe execution memory context of the surrounding SPI (which happens to be pl/pgsql's SPI). This, in effect, frees allthe memory allocated by pl/pgsql for the prepare stage; and the next time the pl/pgsql's data-structures are referenced,it causes a crash. The code in question is: <br /><br />void<br />AtEOSubXact_SPI(bool isCommit, SubTransactionIdmySubid)<br />{<br /> bool found = false;<br /><br /> while (_SPI_connected >= 0)<br /> {<br /> _SPI_connection *connection = &(_SPI_stack[_SPI_connected]); <br /><br /> if (connection->connectSubid!= mySubid)<br /> break; /* couldn't be any underneath it either */<br/><br /> ......<br /> ......<br /> /*<br /> * If we are aborting a subtransaction and there isan open SPI context <br /> * surrounding the subxact, clean up to prevent memory leakage.<br /> */<br /> if(_SPI_current && !isCommit)<br /> {<br /> /* free Executor memory the same as _SPI_end_call would do*/<br /> MemoryContextResetAndDeleteChildren(_SPI_current->execCxt); <br /> ^^^^^^^^^^^<br /> /*throw away any partially created tuple-table */<br /> SPI_freetuptable(_SPI_current->tuptable);<br /> _SPI_current->tuptable = NULL;<br /> }<br />}<br /><br /> The code is doing what the comments say, buthow I wish it didn't. This code assumes that the latest BIST() came from a SPI invoking module, which the Index Adviseris not! <br /><br /> I haven't been able to come up with a proof, but I think this situation can be simulated/reproducedusing some pl/* language that we support.<br /><br /> pl/pgsql is safe, since it doesn't allow SAVEPOINT/ROLLBACKTO SAVEPOINT inside it's code blocks; but if some pl/xxx allows SAVEPOINTS, I think it can be reproducedusing something like: <br /><br /> SAVEPOINT xxx ( should call BIST() )<br /> SAVEPOINT yyy ( should call BIST() )<br /> Rollback (to yyy) ( should call RARCST() ) will free pl/xxx exec context<br/> Rollback (to xxx) ( should call RARCST() ) <br /><br /> Probably just one pair, instead of twonested ones, should reproduce it. I tried to reproduce it using pl/pgsql's exception block (there we use BIST() and RARCST());but couldn't succeed.<br /><br /> I hope I have understood the issue correctly, or have I missed something?Is there a way to work around this? <br /><br />Best regards,<br clear="all" /><span class="sg"><br />-- <br />gurjeet[.singh]@EnterpriseDB.com<br />singh.gurjeet@{ gmail | hotmail | yahoo }.com </span></blockquote></div>
On Sun, 2006-12-03 at 17:55 +0530, Gurjeet Singh wrote: > Can the memory leaks be avoided in some other way? Possibly, but it's not likely to happen just to help you out here unfortuantely. There's another way, I'm sure -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On 12/4/06, Simon Riggs <simon@2ndquadrant.com> wrote:
That's sad... well, I have devised a workaround, and I am sure that even that isn't pretty enough to be liked by everyone.
--
gurjeet[.singh]@ EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
On Sun, 2006-12-03 at 17:55 +0530, Gurjeet Singh wrote:
> Can the memory leaks be avoided in some other way?
Possibly, but it's not likely to happen just to help you out here
unfortuantely.
That's sad... well, I have devised a workaround, and I am sure that even that isn't pretty enough to be liked by everyone.
There's another way, I'm sure
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
--
gurjeet[.singh]@ EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
<span style="font-family: courier new,monospace;">Another question.<br /><br />There are two global variables:<br /><br />CurrentResourceOwner(CRO) and CurrentTransactionState (CTS). CTS has a member curTransactionOwner(cTO). Now the questionis: <br /><br />is the following condition always supposed to be true?<br /><br />CurrentResourceOwner==CurrentTransactionState->curTransactionOwner<br/><br /> While in executor, CRO->name is"Portal". Then I do the BeginInternalSubTransaction() (BIST()); it pushes the current (sub)transaction on a stack and andalong with it, it sets the CTS as it's parent. CTS->cTO->name is "TopTransaction". And a new resource-owner is assignedto the new SubTransaction by AtSubStart_ResourceOwner(). <br /><br /> After the adviser is done with it's work,it calls RollbackAndReleaseCurrentSubTransaction() (RARCST()), which restores the global CRO to the previously pushedSubTransaction's parent; this is "TopTransaction". Now, we have lost record of which was the ResourceOwner before BIST()and erroneously restored "TopTransaction" to be the CRO. This is causing some diffcult to track ERROR conditions. <br/><br /> So, is the above mentioned condition supposed to hold always? (I am assuming not). If not, then we shouldremember the CurrentResourceOwner across BIST() and RARCST() calls!<br /><br /> Sorry to be pestering you guys withseemingly trivial stuff, but you would also agree that it becomes very difficult to understand the code when there areso many globals and the relationship between them is not very clear. <br /><br />Regards,<br /></span><span style="font-family:courier new,monospace;"><br style="font-family: courier new,monospace;" /></span><span style="font-family:courier new,monospace;">-- </span><br style="font-family: courier new,monospace;" /><span style="font-family:courier new,monospace;">gurjeet[.singh]@EnterpriseDB.com</span><br style="font-family: courier new,monospace;"/><span style="font-family: courier new,monospace;">singh.gurjeet@{ gmail | hotmail | yahoo }.com </span>
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes: > is the following condition always supposed to be true? > CurrentResourceOwner==CurrentTransactionState->curTransactionOwner No. > If not, then we should remember the CurrentResourceOwner > across BIST() and RARCST() calls! As indeed the current callers of them do... regards, tom lane
On 12/4/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thanks. I missed that piece of code in pl/pgsql exception handling!!!
So, if I use kludge the code like:
oldResourceOwner = CurrentResourceOwner;
BIST();
... do stuff ...
RARCST();
CurrentResourceOwner = oldResourceOwner;
it would be a standard way of doing it.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
> we should remember the CurrentResourceOwner
> across BIST() and RARCST() calls!
As indeed the current callers of them do...
Thanks. I missed that piece of code in pl/pgsql exception handling!!!
So, if I use kludge the code like:
oldResourceOwner = CurrentResourceOwner;
BIST();
... do stuff ...
RARCST();
CurrentResourceOwner = oldResourceOwner;
it would be a standard way of doing it.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com