Subject: Re: [POC] Fast COPY FROM command for the table with foreign
 partitions
To: Amit Langote <amitlangote09@gmail.com>
Cc: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>,
 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
 Etsuro Fujita <etsuro.fujita@gmail.com>,
 PostgreSQL-Dev <pgsql-hackers@postgresql.org>
References: <3d0909dc-3691-a576-208a-90986e55489f@postgrespro.ru>
 <CAPmGK15mAiaU85OcvEDE=7fHA3PvA7ChP82b7bEE57Yw20rF4A@mail.gmail.com>
 <7a3c3bdd-dc43-68a7-7cc5-ae139f1de4a7@postgrespro.ru>
 <CAExHW5uAtyAVL-iuu1Hsd0fycqS5UHoHCLfauYDLQwRucwC9Og@mail.gmail.com>
 <bd29904a-4ad3-3619-eb0c-ff04f524f7a1@postgrespro.ru>
 <CAG-ACPUZ1FRwkRfOnBZN0PYvXSZrBRgaExyRT4QHaN20rf7A8Q@mail.gmail.com>
 <ae25e79e-6122-720e-1349-fb832c6021df@postgrespro.ru>
 <CA+HiwqH2ehGurL9bYUydXBxVUH6aHN5yXXxESAOs9O6-f_+Qeg@mail.gmail.com>
 <cf1e92eb-9a8e-f5c1-f0cc-598882f1e546@postgrespro.ru>
 <CA+HiwqHRqG8X9W6-v-E6d0nw1k6aC7QSMk-SOvv4puOjWsQMmA@mail.gmail.com>
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
Message-ID: <9a65d03b-250f-b086-5a3c-541d8e2de30d@postgrespro.ru>
Date: Wed, 29 Jul 2020 13:36:01 +0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: 
 <CA+HiwqHRqG8X9W6-v-E6d0nw1k6aC7QSMk-SOvv4puOjWsQMmA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/29/20 1:03 PM, Amit Langote wrote:
> Hi Andrey,
> 
> Thanks for updating the patch.  I will try to take a look later.
> 
> On Wed, Jul 22, 2020 at 6:09 PM Andrey V. Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
>> On 7/16/20 2:14 PM, Amit Langote wrote:
>>> * Why the "In" in these API names?
>>>
>>> +   /* COPY a bulk of tuples into a foreign relation */
>>> +   BeginForeignCopyIn_function BeginForeignCopyIn;
>>> +   EndForeignCopyIn_function EndForeignCopyIn;
>>> +   ExecForeignCopyIn_function ExecForeignCopyIn;
>>
>> I used an analogy from copy.c.
> 
> Hmm, if we were going to also need *ForeignCopyOut APIs, maybe it
> makes sense to have "In" here, but maybe we don't, so how about
> leaving out the "In" for clarity?
Ok, sounds good.
> 
>>> * I see that the remote copy is performed from scratch on every call
>>> of postgresExecForeignCopyIn(), but wouldn't it be more efficient to
>>> send the `COPY remote_table FROM STDIN` in
>>> postgresBeginForeignCopyIn() and end it in postgresEndForeignCopyIn()
>>> when there are no errors during the copy?
>>
>> It is not possible. FDW share one connection between all foreign
>> relations from a server. If two or more partitions will be placed at one
>> foreign server you will have problems with concurrent COPY command.
> 
> Ah, you're right.  I didn't consider multiple foreign partitions
> pointing to the same server.  Indeed, we would need separate
> connections to a given server to COPY to multiple remote relations on
> that server in parallel.
> 
>> May be we can create new connection for each partition?
> 
> Yeah, perhaps, although it sounds like something that might be more
> generally useful and so we should work on that separately if at all.
I will try to prepare a separate patch.
> 
>>> I tried implementing these two changes -- pgfdw_copy_data_dest_cb()
>>> and sending `COPY remote_table FROM STDIN` only once instead of on
>>> every flush -- and I see significant speedup.  Please check the
>>> attached patch that applies on top of yours.
>>
>> I integrated first change and rejected the second by the reason as above.
> 
> Thanks.
> 
> Will send more comments after reading the v5 patch.
> 
Ok. I'll be waiting for the end of your review.

-- 
regards,
Andrey Lepikhov
Postgres Professional