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