From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: plan cache overhead on plpgsql expression
Date: Thu, 26 Mar 2020 16:59:49 -0400
Message-ID: <21695.1585256389@sss.pgh.pa.us> (raw)
In-Reply-To: <20200326184947.vniqm2kqa4hmv56x@alap3.anarazel.de>
References: <CA+HiwqHuEmzTnYZaJp8D14VCc127dnAKdAWZoV-b6U7oO_oOJg@mail.gmail.com>
<CA+HiwqEFt+cJUi=DLxmynYwFwWrC3mQMUSMcYefxU-5jgaeWew@mail.gmail.com>
<CAFj8pRBVeseNF+jDaZOrC0-W540Mx-DAEvELMeveJs10L9i=Rg@mail.gmail.com>
<CAFj8pRBH2T=7VoTrY7hf5MNUWE45zZpdWWvv+hzEem=Wn3GcNg@mail.gmail.com>
<1478.1584815045@sss.pgh.pa.us>
<20200325211825.gf2ta424kwkfghbr@alap3.anarazel.de>
<15634.1585173110@sss.pgh.pa.us>
<20200325224903.pwsx4pccyktq7tx6@alap3.anarazel.de>
<11118.1585231340@sss.pgh.pa.us>
<10353.1585247879@sss.pgh.pa.us>
<20200326184947.vniqm2kqa4hmv56x@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> On 2020-03-26 14:37:59 -0400, Tom Lane wrote:
>> Testing that reminded me of the other regression test failure I'd seen
>> when I first tried to do it: select_parallel.sql shows a WARNING about
>> a plancache leak in a parallel worker process. When I looked into the
>> reason for that, it turned out that some cowboy has split
>> XACT_EVENT_COMMIT into XACT_EVENT_COMMIT and
>> XACT_EVENT_PARALLEL_COMMIT (where the latter is used in parallel
>> workers) without bothering to fix the collateral damage to plpgsql.
>> So plpgsql_xact_cb isn't doing any cleanup in parallel workers, and
>> hasn't been for a couple of releases.
>> The bad effects of that are probably limited given that the worker
>> process will exit after committing, but I still think that that part
>> of this patch is a bug fix that needs to be back-patched.
> Ugh. Lucky that we don't register many things inside those resowners.
Yeah. I spent some time trying to produce a failure this way, and
concluded that it's pretty hard because most of the relevant callbacks
will be run during ExprContext shutdown, which is done during plpgsql
function exit. In a non-transaction-abort situation, the simple EState
shouldn't have any live ExprContexts left at commit. I did find a case
where a memory context callback attached to the EState's query context
doesn't get run when expected ... but it still gets run later, when the
whole memory context tree is destroyed. So I can't demonstrate any
user-visible misbehavior in the core code. But it still seems like a
prudent idea to back-patch a fix, in case I missed something or there is
some extension that's pushing the boundaries further. It's definitely
not very cool that we're leaving behind a dangling static pointer to an
EState that won't exist once TopTransactionMemoryContext is gone.
I'll back-patch relevant parts of those comments about DO block
management, too.
regards, tom lane
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-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, pavel.stehule@gmail.com, amitlangote09@gmail.com
Subject: Re: plan cache overhead on plpgsql expression
In-Reply-To: <21695.1585256389@sss.pgh.pa.us>
* 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