Re: Asynchronous Append on postgres_fdw nodes.

From: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pryzby(at)telsasoft(dot)com
Cc: a(dot)lepikhov(at)postgrespro(dot)ru, etsuro(dot)fujita(at)gmail(dot)com, movead(dot)li(at)highgo(dot)ca, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Asynchronous Append on postgres_fdw nodes.
Date: 2020-09-22 13:40:11
Message-ID: af8e10c8-f893-1a5a-483e-9ae4f4a7b718@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 22.09.2020 15:52, Konstantin Knizhnik wrote:
>
>
> On 20.08.2020 10:36, Kyotaro Horiguchi wrote:
>> At Wed, 19 Aug 2020 23:25:36 -0500, Justin Pryzby
>> <pryzby(at)telsasoft(dot)com> wrote in
>>> On Thu, Jul 02, 2020 at 11:14:48AM +0900, Kyotaro Horiguchi wrote:
>>>> As the result of a discussion with Fujita-san off-list, I'm going to
>>>> hold off development until he decides whether mine or Thomas' is
>>>> better.
>>> The latest patch doesn't apply so I set as WoA.
>>> https://commitfest.postgresql.org/29/2491/
>> Thanks. This is rebased version.
>>
>> At Fri, 14 Aug 2020 13:29:16 +1200, Thomas Munro
>> <thomas(dot)munro(at)gmail(dot)com> wrote in
>>> Either way, we definitely need patch 0001.  One comment:
>>>
>>> -CreateWaitEventSet(MemoryContext context, int nevents)
>>> +CreateWaitEventSet(MemoryContext context, ResourceOwner res, int
>>> nevents)
>>>
>>> I wonder if it's better to have it receive ResourceOwner like that, or
>>> to have it capture CurrentResourceOwner.  I think the latter is more
>>> common in existing code.
>> There's no existing WaitEventSets belonging to a resowner. So
>> unconditionally capturing CurrentResourceOwner doesn't work well. I
>> could pass a bool instead but that make things more complex.
>>
>> Come to think of "complex", ExecAsync stuff in this patch might be
>> too-much for a short-term solution until executor overhaul, if it
>> comes shortly. (the patch of mine here as a whole is like that,
>> though..). The queueing stuff in postgres_fdw is, too.
>>
>> regards.
>>
>
>
> Hi,
> Looks like current implementation of asynchronous append incorrectly
> handle LIMIT clause:
>
> psql:append.sql:10: ERROR:  another command is already in progress
> CONTEXT:  remote SQL command: CLOSE c1
>
>
>
Just FYI: the following patch fixes the problem:

--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1667,6 +1667,11 @@ remove_async_node(ForeignScanState *node)

         if (cur == node)
         {
+            PGconn *conn = curstate->s.conn;
+
+            while(PQisBusy(conn))
+                PQclear(PQgetResult(conn));
+
             prev_state->waiter = curstate->waiter;

             /* relink to the previous node if the last node was removed */

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
async_append_limit.patch text/x-patch 505 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-09-22 14:18:40 Re: pgindent vs dtrace on macos
Previous Message Ashutosh Bapat 2020-09-22 12:55:57 Re: Transactions involving multiple postgres foreign servers, take 2