Horton

Thread

Commits

  1. Handle interrupts while waiting on Append's async subplans

  1. Handle interrupts while waiting on Append's async subplans

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-03-03T17:44:45Z

    While working on the big interrupts refactoring, I noticed that 
ExecAppendAsyncEventWait() doesn't include WL_LATCH_SET in the wait set. 
As a result, it will not wake up on an interrupt like a query cancel 
request or sinval catchup.

I'm surprised this has gone unnoticed for so long. It's pretty easy to 
reproduce the issue with a foreign table over a view on "SELECT 
pg_sleep(100)", for example. Did I miss a bug report?

At first glance the fix is very straightforward; we just need to call 
AddWaitEventToSet() to wait for WL_LATCH_SET too. However, when I did 
that the straightforward way, the 'postgres_fdw' test got stuck in a 
busy-loop in ExecAppendAsyncGetNext(). 
postgresForeignAsyncConfigureWait() does this:

> 	else if (pendingAreq->requestor != areq->requestor)
> 	{
> 		/*
> 		 * This is the case when the in-process request was made by another
> 		 * Append.  Note that it might be useless to process the request made
> 		 * by that Append, because the query might not need tuples from that
> 		 * Append anymore; so we avoid processing it to begin a fetch for the
> 		 * given request if possible.  If there are any child subplans of the
> 		 * same parent that are ready for new requests, skip the given
> 		 * request.  Likewise, if there are any configured events other than
> 		 * the postmaster death event, skip it.  Otherwise, process the
> 		 * in-process request, then begin a fetch to configure the event
> 		 * below, because we might otherwise end up with no configured events
> 		 * other than the postmaster death event.
> 		 */
> 		if (!bms_is_empty(requestor->as_needrequest))
> 			return;
> 		if (GetNumRegisteredWaitEvents(set) > 1)
> 			return;
> 		process_pending_request(pendingAreq);
> 		fetch_more_data_begin(areq);
> 	}

When I added the WL_LATCH_SET event to the set before calling this, the 
check for "GetNumRegisteredWaitEvents(set) > 1" was always true.

I don't understand what that comment is trying to say. What is an 
"in-process request"? And why does postgres_fdw care what other async 
subplans are registered in the Append node? If it is legitimate for it 
to care, we probably should've abstracted that somehow, rather than 
expose that "GetNumRegisteredWaitEvents(set) == 1" means that there are 
no other events registered yet.

In any case, the attached patch works and seems like an easy fix for 
stable branches at least.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

  2. Re: Handle interrupts while waiting on Append's async subplans

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-03-12T19:16:01Z

    On 03/03/2025 19:44, Heikki Linnakangas wrote:
> In any case, the attached patch works and seems like an easy fix for 
> stable branches at least.

Committed that to master and all affected stable branches.

-- 
Heikki Linnakangas
Neon (https://neon.tech)