Date: Tue, 17 Sep 2024 22:30:26 -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: <20240918053026.79.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>
 <20240918050434.13.nmisch@google.com>
 <1744648.1726636728@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1744648.1726636728@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240918053026.79.nmisch%40google.com>
Precedence: bulk

On Wed, Sep 18, 2024 at 01:18:48AM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Wed, Sep 18, 2024 at 12:23:42AM -0400, Tom Lane wrote:
> >> 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?
> 
> > 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?
> 
> Ugh.  Probably not very.  We have to be able to perform comparison
> operations within the scanning of a page.  Even if it could be made
> to work correctly to release and re-acquire the buffer lock many
> times per page, it sounds awful for performance.

Sounds that way.

> > 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?
> 
> Dunno ... but the OP claimed this is a case that's seen in the
> field, so maybe somebody is doing it.  On the whole I don't see
> how a btree support function can be considered not to be a low-level
> thing, so maybe restricting what it can do is the best answer.
> I fear though that the restriction can't simply be to forbid
> parallel sub-queries.
> 
> On the third hand: you can't implement a btree comparison function
> in SQL and simultaneously claim with a straight face that you
> expect premium performance.  Could we set things up so that
> buffer release/reacquires happen only with non-C support functions?
> This still requires that such behavior is safe in the face of
> concurrent index activity, which I'm not sure is practical.
> (Also, I'm not sure to what extent we'd still be testing what
> we wish to with test comparison functions written in SQL.)

Not to rule it out yet, but your point about reduced test fidelity is cause
for hesitation.  Also, I'd regret even having that code bloating up a btree
hot path.

What if we just ignored the plancache when uninterruptible?  The new planner
check would then suffice.  If it slows anything folks care about, either find
a way to become interruptible or make a C function that bypasses planner &
executor.