Re: Bug in asynchronous Append - Mailing list pgsql-hackers

From Alexander Pyhalov
Subject Re: Bug in asynchronous Append
Date
Msg-id d15d8283c5960e84af984b2bf3808497@postgrespro.ru
Whole thread
In response to Bug in asynchronous Append  (Alexander Korotkov <aekorotkov@gmail.com>)
Responses Re: Bug in asynchronous Append
List pgsql-hackers
Etsuro Fujita писал(а) 2026-08-03 16:57:
> Hi,
> 
> On Mon, Aug 3, 2026 at 6:45 PM Alexander Pyhalov
> <a.pyhalov@postgrespro.ru> wrote:
>> I've looked on the suggested patch more attentively in Async Merge
>> Append thread[1].
>> There it's a bit more obvious that ExecReScanAppend() behavior in
>> ExecAppendAsyncProcessPending()
>> depends on node->as_syncdone. It either sleeps on latch in
>> ExecAppendAsyncEventWait() or busy loops.
>> The first behavior seems to be more appropriate, as we still should 
>> wait
>> for all requests
>> with set callback_pending to complete. Or perhaps, it's not a big
>> problem, given that this case should
>> be rare?
> 
> Good catch!  Incorporated.  Attached is an updated version of the
> patch.  Other changes are:
> 
> * Add CHECK_FOR_INTERRUPTS() to the for loop in 
> ExecAppendAsyncProcessPending.
> * Merge the async-state-reset code in ExecReScanAppend into
> ExecAppendAsyncProcessPending, for readability, and rename that
> function to ExecAppendAsyncReset.
> * Remove this bit from postgresReScanForeignScan:
> 
>     /*
>      * If the node is async-capable, and an asynchronous fetch for it 
> has
>      * begun, the asynchronous fetch might not have yet completed.  
> Check if
>      * the node is async-capable, and an asynchronous fetch for it is 
> still in
>      * progress; if so, complete the asynchronous fetch before 
> restarting the
>      * scan.
>      */
>     if (fsstate->async_capable &&
>         fsstate->conn_state->pendingAreq &&
>         fsstate->conn_state->pendingAreq->requestee == (PlanState *) 
> node)
>         fetch_more_data(node);
> 
> and instead add an assertion, as it's no longer needed due to the
> handling in ExecAppendAsyncReset.
> 
> * As a test case causing an issue (infinite loop!) on a
> non-assert-enabled build, add (a modified version of) Gleb's test case
> as well.  (The original test case only causes an assertion failure.)
> 

Hi. Looks good to me.
-- 
Best regards,
Alexander Pyhalov,
Postgres Professional



pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: Add more tab=completion rules for DROP PROPERTY GRAPH
Next
From: Daniel Gustafsson
Date:
Subject: Re: data_checksums + debug_discard_caches = crash