Re: Append with naive multiplexing of FDWs

On Tue, Sep 1, 2020 at 9:45 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
> At Mon, 31 Aug 2020 19:10:39 +0900, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote in
> > On Mon, Aug 31, 2020 at 6:20 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> > > * I know your patch is a POC one, but one concern about it (and
> > > Horiguchi-san's patch set) is concurrent data fetches by multiple
> > > foreign scan nodes using the same connection in the case of
> > > postgres_fdw. Here is an example causing an error:
> >
> > > select * from pt1, pt2 where pt2.a = 't22' or pt2.a = 't23';
> > > ERROR: another command is already in progress
> > > CONTEXT: remote SQL command: DECLARE c4 CURSOR FOR
> > > SELECT a, b FROM public.t22 WHERE (((a = 't22'::text) OR (a = 't23'::text)))
> >
> > > (Horiguchi-san’s patch set doesn't work for this query either, causing
> > > the same error. Though, it looks like he intended to handle cases
> > > like this by a queuing system added to postgres_fdw to process such
> > > concurrent data fetches.)
> >
> > I was wrong here; Horiguchi-san's patch set works well for this query.
> > Maybe I did something wrong when testing his patch set. Sorry for
> > that.
>
> Yeah. postgresIterateForeignScan calls vacate_connection() to make the
> underlying connection available if a server connection is busy with
> another remote query. The mechanism is backed by a waiting queue
> (add_async_waiter, move_to_next_waiter, remove_async_node).

Thanks for the explanation, Horiguchi-san!

So your version of the patch processes the query successfully, because
1) before performing an asynchronous data fetch of ft22, it waits for
the in-progress data fetch of ft12 using the same connection to
complete so that the data fetch of ft22 can be done, and 2) before
performing an asynchronous data fetch of ft23, it waits for the
in-progress data fetch of ft13 using the same connection to complete
so that the data fetch of ft23 can be done. Right? If so, I think in
some cases such handling would impact performance negatively.
Consider the same query with LIMIT processed by your version:

explain verbose select * from pt1, pt2 where pt2.a = 't22' or pt2.a =
't23' limit 1;
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
Limit (cost=200.00..200.01 rows=1 width=128)
Output: pt1.a, pt1.b, pt2.a, pt2.b
-> Nested Loop (cost=200.00..903.87 rows=50220 width=128)
Output: pt1.a, pt1.b, pt2.a, pt2.b
-> Append (cost=100.00..151.85 rows=2790 width=64)
Async subplans: 3
-> Async Foreign Scan on public.ft11 pt1_1
(cost=100.00..137.90 rows=930 width=64)
Output: pt1_1.a, pt1_1.b
Remote SQL: SELECT a, b FROM public.t11
-> Async Foreign Scan on public.ft12 pt1_2
(cost=100.00..137.90 rows=930 width=64)
Output: pt1_2.a, pt1_2.b
Remote SQL: SELECT a, b FROM public.t12
-> Async Foreign Scan on public.ft13 pt1_3
(cost=100.00..137.90 rows=930 width=64)
Output: pt1_3.a, pt1_3.b
Remote SQL: SELECT a, b FROM public.t13
-> Materialize (cost=100.00..124.31 rows=18 width=64)
Output: pt2.a, pt2.b
-> Append (cost=100.00..124.22 rows=18 width=64)
Async subplans: 2
-> Async Foreign Scan on public.ft22 pt2_1
(cost=100.00..124.13 rows=9 width=64)
Output: pt2_1.a, pt2_1.b
Remote SQL: SELECT a, b FROM public.t22
WHERE (((a = 't22'::text) OR (a = 't23'::text)))
-> Async Foreign Scan on public.ft23 pt2_2
(cost=100.00..124.13 rows=9 width=64)
Output: pt2_2.a, pt2_2.b
Remote SQL: SELECT a, b FROM public.t23
WHERE (((a = 't22'::text) OR (a = 't23'::text)))
(25 rows)

I think your version would require extra time to process this query
compared to HEAD due to such handling. This query throws an error
with your version, though:

select * from pt1, pt2 where pt2.a = 't22' or pt2.a = 't23' limit 1;
ERROR: another command is already in progress
CONTEXT: remote SQL command: CLOSE c1

Best regards,
Etsuro Fujita