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
