Date: Tue, 17 Sep 2024 22:04:34 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Francesco Degrassi <francesco.degrassi@optionfactory.net>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: Leader backend hang on IPC/ParallelFinish when LWLock held at
 parallel query start
Message-ID: <20240918050434.13.nmisch@google.com>
References: 
 <CAC-SaSzHUKT=vZJ8MPxYdC_URPfax+yoA1hKTcF4ROz_Q6z0_Q@mail.gmail.com>
 <20240918030159.2a.nmisch@google.com>
 <1737570.1726633422@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1737570.1726633422@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240918050434.13.nmisch%40google.com>
Precedence: bulk

On Wed, Sep 18, 2024 at 12:23:42AM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Mon, Sep 16, 2024 at 09:35:13PM +0200, Francesco Degrassi wrote:
> >> The problem appears to manifest when a backend is holding an LWLock and
> >> starting a query, and the planner chooses a parallel plan for the
> >> latter.
> 
> > Thanks for the detailed report and for the fix.
> 
> I do not have much faith in this patch.  It assumes that the
> condition "interrupts can be processed" is the same at plan time and
> execution time.  For plans extracted from the plan cache, there seems
> little reason to assume that must be true.  The proposed test case
> cannot trigger that (today anyway) because SQL-language functions
> don't deal in cached plans, but I suspect it could be broken with a
> test case using a plpgsql function instead.

Good point.  I missed that.

> I actually think that the problem is somewhere upstream of here:
> what in the world are we doing invoking planning and execution of
> arbitrary queries in a context where interrupts are held off?
> That seems certain to fail in multiple ways besides parallel
> workers not working.  I think we need to fix whatever code
> believes that that could be OK.  And maybe then insert
> "Assert(INTERRUPTS_CAN_BE_PROCESSED())" at planner start
> and executor start.

It would be nice to never run planning or execution with interrupts held.  The
concrete examples so far involve btree operator classes with non-C support
functions.  How practical would it be to release buffer content locks before
running index support functions?  An alternative would be blocking non-C
support functions (and instructing C support function authors to not use
planner/executor).  Non-C support functions have been handy for testing if
nothing else, though.  Do non-bundled modules rely on non-C support functions?

Thanks,
nm