From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: Memory leak in RelationBuildRowSecurity
Date: Sat, 26 Sep 2020 19:37:45 +0300
Message-ID: <c28d5597-1933-1933-c15c-edadd9186757@postgrespro.ru> (raw)
In-Reply-To: <1937613.1601057234@sss.pgh.pa.us>
References: <21356c12-8917-8249-b35f-1c447231922b@postgrespro.ru>
<1852787.1601041699@sss.pgh.pa.us>
<3f670fa6-5e81-fbd9-b880-eaa275f15e1c@postgrespro.ru>
<1937613.1601057234@sss.pgh.pa.us>
On 25.09.2020 21:07, Tom Lane wrote:
> Konstantin Knizhnik <k.knizhnik@postgrespro.ru> writes:
>> On 25.09.2020 16:48, Tom Lane wrote:
>>> Hm ... this smells a whole lot like the issue we fixed recently for
>>> partition expressions, namely: what the devil are we doing reloading
>>> these expressions during RelationClearRelation? We should delay
>>> populating that cache until the value is requested.
>> Sorry, this stack trace was obtained at 9.6 version of Postgres which
>> the customer is using.
>> So may be the problem with cache invalidation is already fixed.
> Not sure if we've changed anything much in that area. I've occasionally
> wondered if we should not increase the size of the sinval message queue,
> but it hasn't happened yet.
>
> Anyway, looking more closely at RelationBuildRowSecurity, it doesn't
> seem to be doing anything more dangerous than RelationBuildRuleLock
> is; it's definitely not risking recursion, as the partition stuff did.
> So I take back the idea that we need to postpone it till the data is
> referenced. However, it is leaking memory.
>
> Attached is a proposed rewrite that borrows some ideas from
> RelationBuildRuleLock, and also makes use of MemoryContextSetParent
> so we don't need a PG_TRY block. In a very simple test
> (using regress_rls_schema.rls_tbl from the regression database),
> this doesn't seem to leak any memory in the caller context,
> and it also doesn't make the rscxt any bigger than it was before.
>
>> But at least calling pfree for a tree is very strange idea: we should
>> not do it all or allocate tree in separate memory context.
> Yeah, that's pretty useless. But also I think the string forms of
> the expressions are leaking as much memory as the tree forms.
>
> This is against HEAD but I think it wouldn't be hard to back-patch.
> Are you in a position to see if it fixes the problem in your customer's
> environment?
Thank you.
Your patch fixes the problem.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: k.knizhnik@postgrespro.ru, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: Memory leak in RelationBuildRowSecurity
In-Reply-To: <c28d5597-1933-1933-c15c-edadd9186757@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox