From: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
---|---|
To: | David Zhang <david(dot)zhang(at)highgo(dot)ca> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit |
Date: | 2022-05-06 10:08:01 |
Message-ID: | CAPmGK16Kg2Bf90sqzcZ4YM5cN_G-4h7wFUS01qQpqNB+2BG5_w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, May 5, 2022 at 6:39 AM David Zhang <david(dot)zhang(at)highgo(dot)ca> wrote:
> On 2022-05-02 1:25 a.m., Etsuro Fujita wrote:
> > On Wed, Apr 20, 2022 at 4:55 AM David Zhang <david(dot)zhang(at)highgo(dot)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
Good to know! Thanks for testing!
> >> + * 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.
Ok, but I noticed that the comment failed to mention that the
parallel_commit option is disabled by default. Also, I noticed a
comment above it:
* It's enough to determine this only when making new connection because
* all the connections to the foreign server whose keep_connections option
* is changed will be closed and re-made later.
This would apply to the parallel_commit option as well. How about
updating these like the attached? (I simplified the latter comment
and moved it to a more appropriate place.)
Best regards,
Etsuro Fujita
Attachment | Content-Type | Size |
---|---|---|
update-comment-in-make_new_connection.patch | application/octet-stream | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2022-05-06 10:31:23 | Re: strange slow query - lost lot of time somewhere |
Previous Message | Robert Haas | 2022-05-06 09:43:24 | Re: Configuration Parameter/GUC value validation hook |