Patch for SPI subtransaction memory leakage

Started by Tom Laneover 19 years ago4 messagespatches
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

I noticed a gripe about plpgsql exceptions leaking memory:
http://archives.postgresql.org/pgsql-performance/2006-11/msg00032.php
(woulda been nice if he'd reported it more formally, but whatever.)

Some experimentation shows that there is indeed an issue, eg this
function leaks about 16K per iteration in HEAD:

create function fooey(int) returns void as $$
begin
for n in 1..$1 loop
begin
perform 1/0;
exception
when others then null;
end;
end loop;
end$$ language plpgsql;

I came up with the attached patch that gets rid of the bulk of the leak
--- this particular example still leaks about 48 bytes/iteration due to
the two palloc's in exec_run_select(), which it doesn't get control back
to free.  It's hard to see dealing with that without a major rewrite of
plpgsql's memory management :-(

While this patch passes all the regression tests, I'm still a bit
nervous about whether it might free data that someone still wants
somewhere. Anyone have any complicated PL functions or handwritten
SPI-using code they'd like to try it with?

regards, tom lane

*** src/backend/executor/spi.c.orig	Tue Oct  3 23:16:22 2006
--- src/backend/executor/spi.c	Fri Nov  3 00:19:35 2006
***************
*** 254,259 ****
--- 254,272 ----
  				(errcode(ERRCODE_WARNING),
  				 errmsg("subtransaction left non-empty SPI stack"),
  				 errhint("Check for missing \"SPI_finish\" calls.")));
+ 
+ 	/*
+ 	 * If we are aborting a subtransaction and there is an open SPI context
+ 	 * surrounding the subxact, clean up to prevent memory leakage.
+ 	 */
+ 	if (_SPI_current && !isCommit)
+ 	{
+ 		/* free Executor memory the same as _SPI_end_call would do */
+ 		MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
+ 		/* throw away any partially created tuple-table */
+ 		SPI_freetuptable(_SPI_current->tuptable);
+ 		_SPI_current->tuptable = NULL;
+ 	}
  }
#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#1)
Re: Patch for SPI subtransaction memory leakage

On Fri, 2006-11-03 at 00:40 -0500, Tom Lane wrote:

I noticed a gripe about plpgsql exceptions leaking memory:
http://archives.postgresql.org/pgsql-performance/2006-11/msg00032.php
(woulda been nice if he'd reported it more formally, but whatever.)

Some experimentation shows that there is indeed an issue, eg this
function leaks about 16K per iteration in HEAD:

So at 10,000 tpm that leaks a GB of RAM every 7 minutes. I'm surprised
we've not had that reported before; wow!

This is still a live request for more input, yes?

Not sure if you got back to the OP on this point, but I'd suggest
raising this on -perform to get more feedback.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#2)
Re: Patch for SPI subtransaction memory leakage

"Simon Riggs" <simon@2ndquadrant.com> writes:

On Fri, 2006-11-03 at 00:40 -0500, Tom Lane wrote:

Some experimentation shows that there is indeed an issue, eg this
function leaks about 16K per iteration in HEAD:

So at 10,000 tpm that leaks a GB of RAM every 7 minutes.

Uh, no, because it's only an intra-function-call leak. My request
for testing the patch is still open though.

regards, tom lane

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: Patch for SPI subtransaction memory leakage

Tom Lane wrote:

"Simon Riggs" <simon@2ndquadrant.com> writes:

On Fri, 2006-11-03 at 00:40 -0500, Tom Lane wrote:

Some experimentation shows that there is indeed an issue, eg this
function leaks about 16K per iteration in HEAD:

So at 10,000 tpm that leaks a GB of RAM every 7 minutes.

Uh, no, because it's only an intra-function-call leak. My request
for testing the patch is still open though.

This will be improved in 8.2.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +