Re: Asynchronous Append on postgres_fdw nodes.

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Asynchronous Append on postgres_fdw nodes.
Date: 2021-05-10 03:03:08
Message-ID: CAPmGK16R=U2ok=H_QnibLS297_vmt68b6bJ8OYJzQ0DxMo+gyw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 7, 2021 at 7:32 PM Andrey Lepikhov
<a(dot)lepikhov(at)postgrespro(dot)ru> wrote:
> Ok, I agree with the approach, but the next test case failed:
>
> EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
> SELECT * FROM (
> (SELECT * FROM f1) UNION ALL (SELECT * FROM f2)
> ) q1 LIMIT 100;
> ERROR: InstrUpdateTupleCount called on node not yet executed
>
> Initialization script see in attachment.

Reproduced. Here is the EXPLAIN output for the query:

explain verbose select * from ((select * from f1) union all (select *
from f2)) q1 limit 100;
QUERY PLAN
--------------------------------------------------------------------------------------
Limit (cost=100.00..104.70 rows=100 width=4)
Output: f1.a
-> Append (cost=100.00..724.22 rows=13292 width=4)
-> Async Foreign Scan on public.f1 (cost=100.00..325.62
rows=6554 width=4)
Output: f1.a
Remote SQL: SELECT a FROM public.l1
-> Async Foreign Scan on public.f2 (cost=100.00..332.14
rows=6738 width=4)
Output: f2.a
Remote SQL: SELECT a FROM public.l2
(9 rows)

When executing the query “select * from ((select * from f1) union all
(select * from f2)) q1 limit 100” in async mode, the remote queries
for f1 and f2 would be sent to the remote at the same time in the
first ExecAppend(). If the result for the remote query for f1 is
returned first, the local query would be processed using the result,
and the remote query for f2 in progress would be processed during
ExecutorEnd() using process_pending_request() (and vice versa). But
in the EXPLAIN ANALYZE case, InstrEndLoop() is called *before*
ExecutorEnd(), and it initializes the instr->running flag, so in that
case, when processing the in-progress remote query in
process_pending_request(), we would call InstrUpdateTupleCount() with
the flag unset, causing this error.

I think a simple fix for this would be just remove the check whether
the instr->running flag is set or not in InstrUpdateTupleCount().
Attached is an updated patch, in which I also updated a comment in
execnodes.h and docs in fdwhandler.sgml to match the code in
nodeAppend.c, and fixed typos in comments in nodeAppend.c.

Thanks for the review and script!

Best regards,
Etsuro Fujita

Attachment Content-Type Size
fix-EXPLAIN-ANALYZE-for-async-capable-nodes-v2.patch application/octet-stream 16.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2021-05-10 03:31:11 Re: [HACKERS] logical decoding of two-phase transactions
Previous Message Etsuro Fujita 2021-05-10 02:20:09 Re: Inherited UPDATE/DELETE vs async execution