Re: Is there a memory leak in commit 8561e48?

On Thu, May 03, 2018 at 08:45:13AM -0400, Peter Eisentraut wrote: > I went with the patch I had posted, since I needed to move ahead with > something. If it turns out to be a problem, we can easily switch it > around. Okay. > As Tom mentioned, in order to grow the SPI stack to where it has a >

Re: Is there a memory leak in commit 8561e48?

On 5/2/18 20:11, Michael Paquier wrote: > On Wed, May 02, 2018 at 07:03:21PM -0400, Tom Lane wrote: >> It's only ~100 bytes per stack level. I think under normal loads >> nobody would notice. If you're worried about cross-transaction >> memory consumption, our various caches tend to be a lot

Re: Is there a memory leak in commit 8561e48?

On Wed, May 02, 2018 at 07:03:21PM -0400, Tom Lane wrote: > It's only ~100 bytes per stack level. I think under normal loads > nobody would notice. If you're worried about cross-transaction > memory consumption, our various caches tend to be a lot worse. Perhaps, that's one reason why people

Re: Is there a memory leak in commit 8561e48?

Michael Paquier writes: > With connection poolers letting the connections to the server be around > for a long time, wouldn't it be an issue to let this much memory live > longer than the transaction context? The deeper the stack, the more > memory consumed, hence the more

Re: Is there a memory leak in commit 8561e48?

On Wed, May 02, 2018 at 05:20:37PM -0400, Tom Lane wrote: > Peter Eisentraut writes: > > Yes, that was the idea. Here is an adjusted patch. > > Looks OK to me as far as the leak issue goes. I have no opinion on > whether this is adequate in respect to

Re: Is there a memory leak in commit 8561e48?

Peter Eisentraut writes: > Yes, that was the idea. Here is an adjusted patch. Looks OK to me as far as the leak issue goes. I have no opinion on whether this is adequate in respect to cleanup-after-error problems. regards, tom lane

Re: Is there a memory leak in commit 8561e48?

On 5/2/18 12:30, Tom Lane wrote: > I'm not particularly. It seems impossible that _SPI_stack could grow > faster than the machine's actual stack, which would mean (on typical > setups) that you couldn't get more than perhaps 10MB of _SPI_stack > before hitting a stack-overflow error. That's a

Re: Is there a memory leak in commit 8561e48?

Peter Eisentraut writes: > On 5/1/18 11:42, Tom Lane wrote: >> Peter Eisentraut writes: >> That seems like an odd way to approach this. Why not just remove the >> reset of _SPI_stack and _SPI_stack_depth, so as to subtract code

Re: Is there a memory leak in commit 8561e48?

Peter Eisentraut writes: > The memory leak can be fixed by adding a pfree(). That seems like an odd way to approach this. Why not just remove the reset of _SPI_stack and _SPI_stack_depth, so as to subtract code rather than adding it --- that is, make it

Re: Is there a memory leak in commit 8561e48?

On Mon, Apr 30, 2018 at 05:10:14PM -0400, Peter Eisentraut wrote: > The memory leak can be fixed by adding a pfree(). > > The example you show can be fixed by doing SPI cleanup in both > transaction abort and exception return to main loop, similar to other > cases that now have to handle these

Re: Is there a memory leak in commit 8561e48?

On 4/27/18 02:44, Andrew Gierth wrote: >> "Tom" == Tom Lane writes: > > Tom> I would bet money that that "_SPI_current->internal_xact" thing is > Tom> wrong/inadequate. > > In particular this looks wrong to me: after doing: > > do $$ > begin > execute 'declare

Re: Is there a memory leak in commit 8561e48?

> "Tom" == Tom Lane writes: Tom> I would bet money that that "_SPI_current->internal_xact" thing is Tom> wrong/inadequate. In particular this looks wrong to me: after doing: do $$ begin execute 'declare foo cursor with hold for select 1/x as a from (values

Re: Is there a memory leak in commit 8561e48?

On Thu, Apr 26, 2018 at 06:36:01PM -0400, Tom Lane wrote: > Uh, no, the memory *isn't* reused later. The coding in AtEOXact_SPI > assumes that it can just null out _SPI_stack because whatever that > might've been pointing to is transaction-lifespan memory and will > go away without extra work

Re: Is there a memory leak in commit 8561e48?

Peter Eisentraut writes: > On 4/19/18 02:10, Michael Paquier wrote: >> You are right. I can easily see the leak if I use for example a >> background worker which connects to a database, and launches many >> transactions in a row. The laziest reproducer I have

Re: Is there a memory leak in commit 8561e48?

On 4/19/18 02:10, Michael Paquier wrote: > On Thu, Apr 19, 2018 at 11:38:09AM +0800, jian.l...@i-soft.com.cn wrote: >> in commit 8561e48, _SPI_stack alloc from TopMemoryContext. But >> AtEOXact_SPI just set _SPI_stack = NULL. Is this a memory leak? > > You are right. I can easily see the leak

Re: Re: Is there a memory leak in commit 8561e48?

On Fri, Apr 20, 2018 at 10:00:38AM +0800, jian.l...@i-soft.com.cn wrote: > what about just free _SPI_stack in AtEOXact_SPI? if the transaction > end was initiated by SPI , AtEOXact_SPI will do nothing. For example: > @@ -283,6 +295,8 @@ AtEOXact_SPI(bool isCommit) >

Re: Re: Is there a memory leak in commit 8561e48?

stack"), errhint("Check for missing \"SPI_finish\" calls."))); + if (_SPI_stack) + pfree(_SPI_stack); From: Michael Paquier Date: 2018-04-20 08:49 To: Postgres hackers CC: Peter Eisentraut; Andrew Dunstan; jian.l...@i-soft.com.cn Subject: Re: Is

Re: Is there a memory leak in commit 8561e48?

On Thu, Apr 19, 2018 at 03:10:42PM +0900, Michael Paquier wrote: > You are right. I can easily see the leak if I use for example a > background worker which connects to a database, and launches many > transactions in a row. The laziest reproducer I have is to patch one of > my bgworkers to

Re: Is there a memory leak in commit 8561e48?

On Thu, Apr 19, 2018 at 11:38:09AM +0800, jian.l...@i-soft.com.cn wrote: > in commit 8561e48, _SPI_stack alloc from TopMemoryContext. But > AtEOXact_SPI just set _SPI_stack = NULL. Is this a memory leak? You are right. I can easily see the leak if I use for example a background worker which

Is there a memory leak in commit 8561e48?

in commit 8561e48, _SPI_stack alloc from TopMemoryContext. But AtEOXact_SPI just set _SPI_stack = NULL. Is this a memory leak?