Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > I noticed that this patch is conflicting with 665d1fa (Logical > replication) so I rebased this. Only executor/Makefile > conflicted.
I was lucky enough to see an infinite loop when using this patch, which I fixed by this change: diff --git a/src/backend/executor/execAsync.c b/src/backend/executor/execAsync.c new file mode 100644 index 588ba18..9b87fbd *** a/src/backend/executor/execAsync.c --- b/src/backend/executor/execAsync.c *************** ExecAsyncEventWait(EState *estate, long *** 364,369 **** --- 364,370 ---- if ((w->events & WL_LATCH_SET) != 0) { + ResetLatch(MyLatch); process_latch_set = true; continue; } Actually _almost_ fixed because at some point one of the following Assert(areq->state == ASYNC_WAITING); statements fired. I think it was the immediately following one, but I can imagine the same to happen in the branch if (process_latch_set) ... I think the wants_process_latch field of PendingAsyncRequest is not useful alone because the process latch can be set for reasons completely unrelated to the asynchronous processing. If the asynchronous node should use latch to signal it's readiness, I think an additional flag is needed in the request which tells ExecAsyncEventWait that the latch was set by the asynchronous node. BTW, do we really need the ASYNC_CALLBACK_PENDING state? I can imagine the async node either to change ASYNC_WAITING directly to ASYNC_COMPLETE, or leave it ASYNC_WAITING if the data is not ready. In addition, the following comments are based only on code review, I didn't verify my understanding experimentally: * Isn't it possible for AppendState.as_asyncresult to contain multiple responses from the same async node? Since the array stores TupleTableSlot instead of the actual tuple (so multiple items of as_asyncresult point to the same slot), I suspect the slot contents might not be defined when the Append node eventually tries to return it to the upper plan. * For the WaitEvent subsystem to work, I think postgres_fdw should keep a separate libpq connection per node, not per user mapping. Currently the connections are cached by user mapping, but it's legal to locate multiple child postgres_fdw nodes of Append plan on the same remote server. I expect that these "co-located" nodes would currently use the same user mapping and therefore the same connection. -- Antonin Houska Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: http://www.postgresql-support.de, http://www.cybertec.at -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers