From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Tomas Vondra" <tv(at)fuzzy(dot)cz> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: failures on barnacle (CLOBBER_CACHE_RECURSIVELY) because of memory leaks |
Date: | 2014-08-13 03:50:18 |
Message-ID: | 24653.1407901818@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> "Tomas Vondra" <tv(at)fuzzy(dot)cz> writes:
>> So after 83 days, the regression tests on barnacle completed,
> Hah, that's perseverance!
>> and it
>> smells like another memory leak in CacheMemoryContext, similar to those
>> fixed in 078b2ed on May 18.
> Ugh, will look.
I've been experimenting by running the create_view test in isolation
(it's not quite self-contained, but close enough) and I find that there
are two memory leaks exposed here:
1. The relcache.c functions that provide on-demand caching, namely
RelationGetIndexList and RelationGetIndexAttrBitmap, are not careful
to free the old values (if any) of the relcache fields they fill.
I think we believed that the old values would surely always be null ...
but that's not necessarily the case when doing a recursive cache reload
of a system catalog, because we might've used/filled the fields while
fetching the data needed to fill them. This results in a session-lifespan
leak of data in CacheMemoryContext, which is what Tomas saw. (I'm not
real sure that this is a live issue for RelationGetIndexAttrBitmap, but
it demonstrably is for RelationGetIndexList.)
2. reloptions.c's parseRelOptions() leaks memory when disassembling a
reloptions array. The leak is in the caller's memory context which
is typically query-lifespan, so under normal circumstances this is not
significant. However, a CLOBBER_CACHE_RECURSIVELY build can call this
an awful lot of times within a single query. The queries in create_view
that operate on views with security_barrier reloptions manage to eat
quite a lot of memory this way.
The attached patch fixes both of these things. I'm inclined to think
it is not worth back-patching because neither effect could amount to
anything noticeable without CLOBBER_CACHE_RECURSIVELY. Anyone think
otherwise?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
more-relcache-leaks.patch | text/x-diff | 2.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2014-08-13 04:18:43 | Re: proposal for 9.5: monitoring lock time for slow queries |
Previous Message | Amit Kapila | 2014-08-13 03:49:31 | Re: replication commands and log_statements |