Re: Patch for SPI subtransaction memory leakage

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;
+ }
}

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

"Simon Riggs" <simon(at)2ndquadrant(dot)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

Tom Lane wrote:
> "Simon Riggs" <simon(at)2ndquadrant(dot)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(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

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