Обсуждение: memory leak in GIN

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.

--
Jaime Casanova                      www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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
leastthe queryCategories array, and   possibly more, on a rescan. That was visible if a GIN index was used in a
nestedloop 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

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