public inbox for [email protected]
help / color / mirror / Atom feedBug in asynchronous Append
3+ messages / 3 participants
[nested] [flat]
* Bug in asynchronous Append
@ 2026-07-03 22:00 Alexander Korotkov <[email protected]>
0 siblings, 2 replies; 3+ messages in thread
From: Alexander Korotkov @ 2026-07-03 22:00 UTC (permalink / raw)
To: pgsql-hackers
Hi!
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own knowledge
for the same fact: PgFdwConnState.pendingAreq – a pointer to "pending async
request" for a given connection. That connection can be shared by several
partitions/foreign tables (postgres_fdw caches one connection per
server+usermapping pair). The blind reset in nodeAppend.c only touches the
local AsyncRequest.callback_pending; it never touches
PgFdwConnState.pendingAreq, which correctly points to the still-dangling
request.
Later, when another partition sharing that same connection gets its own
ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor" via
pgfdw_exec_query(). Before sending any new command on the connection, that
function first drains whatever request is still outstanding on it:
if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);
And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix. The
fix postpones the reset of the callback_pending flag
to ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v1-0001-Fix-corruption-of-async-request-state-on-Append-r.patch (6.4K, ../../CAPpHfduMOTnV5Zj2KGJ7zanL_10QvccZHtPUaDfJvBhsh9axnQ@mail.gmail.com/3-v1-0001-Fix-corruption-of-async-request-state-on-Append-r.patch)
download | inline diff:
From 1c3637f8b986148745ca96685d652227e7bf4bda Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 4 Jul 2026 00:36:45 +0300
Subject: [PATCH v1] Fix corruption of async request state on Append rescan
ExecReScanAppend() unconditionally cleared callback_pending for every
async subplan in as_asyncplans, regardless of whether a request was
genuinely still in flight. For a subplan whose remote fetch had been
sent but not yet consumed, this desynchronized our local bookkeeping
from the async-capable node's own view of the same fact -- e.g.
postgres_fdw's PgFdwConnState.pendingAreq, which tracks the
outstanding request on a possibly shared connection independently of
our AsyncRequest.callback_pending flag.
If that connection is later reused by another subplan (which happens
routinely under runtime partition pruning, once the subplan holding
the stale request is excluded from a round and a sibling sharing its
connection gets its own ReScan), postgres_fdw's pgfdw_exec_query()
drains the still-registered pendingAreq before sending a new command,
and process_pending_request() asserts that its callback_pending flag
is set. Since we had already cleared it, this assertion fails; in a
non-assert build the connection's state is left inconsistent instead.
Fix this by leaving a still-pending request alone in
ExecReScanAppend(), and instead draining it lazily in
ExecAppendAsyncBegin(), right before the request is reused: call
ExecReScan() on the subplan there (letting the async-capable node
settle any outstanding state on its own terms) and only then reset our
bookkeeping. This mirrors the fix already applied to MergeAppend's
async support elsewhere.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---
.../postgres_fdw/expected/postgres_fdw.out | 16 +++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 11 ++++++
src/backend/executor/nodeAppend.c | 36 +++++++++++++++++--
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0805c56cb1b..55e49b7d67f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11761,6 +11761,22 @@ SELECT * FROM result_tbl ORDER BY a;
(3 rows)
DELETE FROM result_tbl;
+-- Check that a pending async request on a shared connection isn't corrupted
+-- when the Append is rescanned with the subplan holding it pruned out. Here
+-- async_p2 and async_p3 share a connection, and per outer row exactly one of
+-- them is pruned while async_p1 (a different connection) always matches; the
+-- inner LIMIT leaves the other shared-connection request outstanding across
+-- the rescan, and reusing that connection for the next round must drain it
+-- rather than trip over stale state.
+SELECT o.x FROM (VALUES (2505), (3505)) o(x),
+ LATERAL (SELECT a FROM async_pt WHERE a = 1505 OR a = o.x LIMIT 1) s
+ORDER BY o.x;
+ x
+------
+ 2505
+ 3505
+(2 rows)
+
-- Test COPY TO when foreign table is partition
COPY async_pt TO stdout; --error
ERROR: cannot copy from foreign table "async_p1"
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 8162c5496bf..7aad91b0718 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -4055,6 +4055,17 @@ INSERT INTO result_tbl SELECT * FROM async_pt WHERE b === 505;
SELECT * FROM result_tbl ORDER BY a;
DELETE FROM result_tbl;
+-- Check that a pending async request on a shared connection isn't corrupted
+-- when the Append is rescanned with the subplan holding it pruned out. Here
+-- async_p2 and async_p3 share a connection, and per outer row exactly one of
+-- them is pruned while async_p1 (a different connection) always matches; the
+-- inner LIMIT leaves the other shared-connection request outstanding across
+-- the rescan, and reusing that connection for the next round must drain it
+-- rather than trip over stale state.
+SELECT o.x FROM (VALUES (2505), (3505)) o(x),
+ LATERAL (SELECT a FROM async_pt WHERE a = 1505 OR a = o.x LIMIT 1) s
+ORDER BY o.x;
+
-- Test COPY TO when foreign table is partition
COPY async_pt TO stdout; --error
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 987358e27fa..6a5a14cd576 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -469,7 +469,21 @@ ExecReScanAppend(AppendState *node)
{
AsyncRequest *areq = node->as_asyncrequests[i];
- areq->callback_pending = false;
+ /*
+ * Leave a request that is still marked as pending a callback
+ * alone: it may genuinely still be in flight, or it may have an
+ * unconsumed result already sitting on a connection shared with
+ * another subplan (as can happen with postgres_fdw). Blindly
+ * clearing callback_pending here would desync our bookkeeping
+ * from the async-capable node's own, which can lead it to
+ * mishandle that connection later (e.g. postgres_fdw asserts that
+ * a request it still considers in-process has callback_pending
+ * set). Such a request is instead drained lazily, right before
+ * it would be reused, in ExecAppendAsyncBegin().
+ */
+ if (areq->callback_pending)
+ continue;
+
areq->request_complete = false;
areq->result = NULL;
}
@@ -915,7 +929,25 @@ ExecAppendAsyncBegin(AppendState *node)
AsyncRequest *areq = node->as_asyncrequests[i];
Assert(areq->request_index == i);
- Assert(!areq->callback_pending);
+
+ /*
+ * This request may still be marked as pending a callback, if
+ * ExecReScanAppend() left it alone because it might have been
+ * genuinely in flight (or had an unconsumed result waiting on a
+ * connection shared with another subplan). Drain it now, before
+ * reusing it: ExecReScan() lets the async-capable node settle any
+ * such outstanding state (e.g. postgres_fdw's
+ * postgresReScanForeignScan() will wait for an in-progress request on
+ * its own connection and consume its result), after which it's safe
+ * to reset our own bookkeeping and issue a fresh request.
+ */
+ if (areq->callback_pending)
+ {
+ ExecReScan(node->appendplans[i]);
+ areq->callback_pending = false;
+ areq->request_complete = false;
+ areq->result = NULL;
+ }
/* Do the actual work. */
ExecAsyncRequest(areq);
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Bug in asynchronous Append
@ 2026-07-04 09:04 Etsuro Fujita <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 0 replies; 3+ messages in thread
From: Etsuro Fujita @ 2026-07-04 09:04 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: pgsql-hackers
Hi Alexander,
On Sat, Jul 4, 2026 at 7:00 AM Alexander Korotkov <[email protected]> wrote:
> ExecReScanAppend() unconditionally resets callback_pending for all AsyncRequests. The problem is that postgres_fdw keeps its own knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer to "pending async request" for a given connection. That connection can be shared by several partitions/foreign tables (postgres_fdw caches one connection per server+usermapping pair). The blind reset in nodeAppend.c only touches the local AsyncRequest.callback_pending; it never touches PgFdwConnState.pendingAreq, which correctly points to the still-dangling request.
>
> Later, when another partition sharing that same connection gets its own ReScan (for instance, its chgParam changed because of the LATERAL parameter, and it already has a cursor open), it sends "CLOSE cursor" via pgfdw_exec_query(). Before sending any new command on the connection, that function first drains whatever request is still outstanding on it:
>
> if (state && state->pendingAreq)
> process_pending_request(state->pendingAreq);
>
> And process_pending_request() starts with:
>
> Assert(areq->callback_pending);
>
> – which fails, because the flag was corrupted some rounds earlier.
>
> The attached patch contains both the reproduction case and the fix. The fix postpones the reset of the callback_pending flag to ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup along with ExecReScan(), which completes the async fetch.
Interesting! Thanks for the report and patch! Will review.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Bug in asynchronous Append
@ 2026-07-06 14:51 Alexander Pyhalov <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Pyhalov @ 2026-07-06 14:51 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: pgsql-hackers
Alexander Korotkov писал(а) 2026-07-04 01:00:
> Hi!
>
> ExecReScanAppend() unconditionally resets callback_pending for all
> AsyncRequests. The problem is that postgres_fdw keeps its own
> knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer
> to "pending async request" for a given connection. That connection
> can be shared by several partitions/foreign tables (postgres_fdw
> caches one connection per server+usermapping pair). The blind reset in
> nodeAppend.c only touches the local AsyncRequest.callback_pending; it
> never touches PgFdwConnState.pendingAreq, which correctly points to
> the still-dangling request.
>
> Later, when another partition sharing that same connection gets its
> own ReScan (for instance, its chgParam changed because of the LATERAL
> parameter, and it already has a cursor open), it sends "CLOSE cursor"
> via pgfdw_exec_query(). Before sending any new command on the
> connection, that function first drains whatever request is still
> outstanding on it:
>
> if (state && state->pendingAreq)
> process_pending_request(state->pendingAreq);
>
> And process_pending_request() starts with:
>
> Assert(areq->callback_pending);
>
> – which fails, because the flag was corrupted some rounds earlier.
>
> The attached patch contains both the reproduction case and the fix.
> The fix postpones the reset of the callback_pending flag to
> ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
> along with ExecReScan(), which completes the async fetch.
>
Hi. The analysis seems correct to me as well as fix.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-07-06 14:51 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-07-03 22:00 Bug in asynchronous Append Alexander Korotkov <[email protected]>
2026-07-04 09:04 ` Etsuro Fujita <[email protected]>
2026-07-06 14:51 ` Alexander Pyhalov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox