Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

Thanks a lot for the patch update.

On 2022-05-02 1:25 a.m., Etsuro Fujita wrote:
> Hi,
>
> On Wed, Apr 20, 2022 at 4:55 AM David Zhang <david.zhang@highgo.ca> wrote:
>> I tried to apply the patch to master and plan to run some tests, but got
>> below errors due to other commits.
> I rebased the patch against HEAD.  Attached is an updated patch.

Applied the patch v8 to master branch today, and the `make check` is OK. 
I also repeated previous performance tests on three virtual Ubuntu 
18.04, and the performance improvement of parallel abort in 10 times 
average is more consistent.

before:

     abort.1 = 2.6344 ms
     abort.2 = 4.2799 ms

after:

     abort.1 = 1.4105 ms
     abort.2 = 2.2075 ms

>> +     * remote server in parallel at (sub)transaction end.
>>
>> Here, I think the comment above could potentially apply to multiple
>> remote server(s).
> I agree on that point, but I think it's correct to say "the remote
> server" here, because we determine this for the given remote server.
> Maybe I'm missing something, so could you elaborate on it?
Your understanding is correct. I was thinking `remote server(s)` would 
be easy for end user to understand but this is a comment in source code, 
so either way is fine for me.
>> Not sure if there is a way to avoid repeated comments? For example, the
>> same comment below appears in two places (line 231 and line 296).
>>
>> +    /*
>> +     * If requested, consume whatever data is available from the socket.
>> +     * (Note that if all data is available, this allows
>> +     * pgfdw_get_cleanup_result to call PQgetResult without forcing the
>> +     * overhead of WaitLatchOrSocket, which would be large compared to the
>> +     * overhead of PQconsumeInput.)
>> +     */
> IMO I think it's OK to have this in multiple places, because 1) IMO it
> wouldn't be that long, and 2) we already duplicated comments like this
> in the same file in v14 and earlier.  Here is such an example in
> pgfdw_xact_callback() and pgfdw_subxact_callback() in that file in
> those versions:
>
>                      /*
>                       * If a command has been submitted to the remote server by
>                       * using an asynchronous execution function, the command
>                       * might not have yet completed.  Check to see if a
>                       * command is still being processed by the remote server,
>                       * and if so, request cancellation of the command.
>                       */
>
> Thanks for reviewing!  Sorry for the delay.
>
> Best regards,
> Etsuro Fujita
-- 
Best regards,
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca