Re: Bug in asynchronous Append

Etsuro Fujita писал(а) 2026-08-03 16:57:
> Hi,
>
> On Mon, Aug 3, 2026 at 6:45 PM Alexander Pyhalov
> <a(dot)pyhalov(at)postgrespro(dot)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