From: Tom Lane <tgl@sss.pgh.pa.us>
To: Noah Misch <noah@leadboat.com>
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
In-reply-to: <20241107192233.94.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> <3441950.1730999841@sss.pgh.pa.us>
 <20241107192233.94.nmisch@google.com>
Comments: In-reply-to Noah Misch <noah@leadboat.com>
	message dated "Thu, 07 Nov 2024 11:22:33 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3459937.1731007759.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 07 Nov 2024 14:29:19 -0500
Message-ID: <3459938.1731007759@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3459938.1731007759%40sss.pgh.pa.us>
Precedence: bulk

Noah Misch <noah@leadboat.com> writes:
> On Thu, Nov 07, 2024 at 12:17:21PM -0500, Tom Lane wrote:
>> While working on the release notes, I noticed that nothing further
>> got done about this concern.  What do you think of adding a test
>> somewhere early in executor startup, to the effect of
>> =

>> 	if (!INTERRUPTS_CAN_BE_PROCESSED())
>> 	    ereport(ERROR,
>> 	            (errmsg("cannot start a query with interrupts disabled")))=
;
>> =

>> It's definitely a band-aid, but it seems better than leaving
>> things at the status quo.

> That would fire in queries that don't error or hang today, so I think th=
at
> would be worse than the status quo.

Good point.

> This thread's previous commit just forced a serial plan.

My concern is that the previous commit forced new plans to be serial,
but did nothing about re-use of an existing plan.

> The executor
> counterpart would look like having the executor do what it does when the=
re are
> no free worker slots.

Ah, that could be a way out.  Stick an INTERRUPTS_CAN_BE_PROCESSED()
call somewhere in there?  That could even allow us to revert the
planner change, which would simplify testing of the executor change.

			regards, tom lane