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.zh...@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


Reply via email to