memory leak in GIN

On Fri, Mar 11, 2016 at 11:40 PM, Jaime Casanova
<jaime.casanova@2ndquadrant.com> wrote:

Hi,

On the spanish list, Felipe de Jesús Molina Bravo, reported a few days
back that a query that worked well in 9.4 consume all memory in 9.5.
With the self contained test he provided us i reproduced the problem
in 9.5 and 9.6dev.

To test, execute:

pba.sql -- to create the tables and populate
query_crash.sql -- this will consume all your memory and crash your
server eventually

If you drop the GIN indexes, the problem disappear.

I used valgrind to try to hunt the memory leak, attached the resulting
log showing the backend that executed the query. And from the little a
could say from the stack trace valgrind showed, the problem is around
ginPostingListDecodeAllSegments() but i don't see any modifications
there.

I bisected it down to:

d88976cfa1302e8dccdcbfe55e9e29faee8c0cdf is the first bad commit
commit d88976cfa1302e8dccdcbfe55e9e29faee8c0cdf
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed Feb 4 17:40:25 2015 +0200

Use a separate memory context for GIN scan keys.

It was getting tedious to track and release all the different things that
form a scan key. We were leaking at least the queryCategories array, and
possibly more, on a rescan. That was visible if a GIN index was used in a
nested loop join. This also protects from leaks in extractQuery method.

No backpatching, given the lack of complaints from the field. Maybe later,
after this has received more field testing.

I won't have a chance to do any further analysis for a while.

Cheers,

Jeff

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jeff Janes <jeff.janes@gmail.com> writes:

I bisected it down to:

d88976cfa1302e8dccdcbfe55e9e29faee8c0cdf is the first bad commit
commit d88976cfa1302e8dccdcbfe55e9e29faee8c0cdf
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed Feb 4 17:40:25 2015 +0200

Use a separate memory context for GIN scan keys.

Yeah, I had come to the same conclusion. The leak comes from removing
this bit from ginFreeScanKeys():

- if (entry->list)
- pfree(entry->list);

Heikki evidently supposed that entry->list would be allocated in
the so->keyCtx, but as things stand, it is not: it gets allocated
in the query-lifespan context, so this change causes a leak of
potentially several kB per ginrescan cycle. And the test query
does about 120000 of those.

I think it would likely be a good thing to fix things so that that
assumption actually holds, but I'm not familiar enough with this code
to decide what's the best way to do that. (Basically, the question is
"how much of startScanEntry() ought to run with keyCtx as current memory
context?") As a short-term fix I plan to reinstall the above-cited pfree.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers