From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: pryzby@telsasoft.com
Cc: a.lepikhov@postgrespro.ru
Cc: etsuro.fujita@gmail.com
Cc: movead.li@highgo.ca
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Asynchronous Append on postgres_fdw nodes.
Date: Tue, 22 Sep 2020 16:40:11 +0300
Message-ID: <af8e10c8-f893-1a5a-483e-9ae4f4a7b718@postgrespro.ru> (raw)
In-Reply-To: <84ea4412-5c0e-9a9c-df04-79033fc066da@postgrespro.ru>
References: <20200610.120510.2075667454883130505.horikyota.ntt@gmail.com>
<20200702.111448.2019475770338322857.horikyota.ntt@gmail.com>
<20200820042536.GB2886@telsasoft.com>
<20200820.163608.1893015081639298019.horikyota.ntt@gmail.com>
<84ea4412-5c0e-9a9c-df04-79033fc066da@postgrespro.ru>
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@telsasoft.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.munro@gmail.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:
inline patch 1: contrib/postgres_fdw/postgres_fdw.c (+5/-0 lines) download inline-1.patch--- 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
Patches and attachments:download all (2 files, .tar.gz)async_append_limit.patch (text/x-patch, 504B) downloaddiff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.cindex 1482436..9fe16cf 100644--- 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 */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: k.knizhnik@postgrespro.ru, horikyota.ntt@gmail.com, pryzby@telsasoft.com, a.lepikhov@postgrespro.ru, etsuro.fujita@gmail.com, movead.li@highgo.ca, pgsql-hackers@lists.postgresql.org
Subject: Re: Asynchronous Append on postgres_fdw nodes.
In-Reply-To: <af8e10c8-f893-1a5a-483e-9ae4f4a7b718@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox