RE: Perform streaming logical transactions by background workers and parallel apply

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
Date: 2022-09-05 13:04:33
Message-ID: OS0PR01MB5716730C06159452335D870D947F9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Monday, September 5, 2022 8:41 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Friday, September 2, 2022 2:10 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Thu, Sep 1, 2022 at 4:53 PM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> >
> > Review of v27-0001*:
>
> Thanks for the comments.
>
> > ================
> > 1. I feel the usage of in_remote_transaction and in_use flags is slightly complex.
> > IIUC, the patch uses in_use flag to ensure commit ordering by waiting
> > for it to become false before proceeding in transaction finish
> > commands in leader apply worker. If so, I think it is better to name
> > it in_parallel_apply_xact and set it to true only when we start
> > applying xact in parallel apply worker and set it to false when we
> > finish the xact in parallel apply worker. It can be initialized to
> > false while setting up DSM. Also, accordingly change the function
> > parallel_apply_wait_for_free() to parallel_apply_wait_for_xact_finish
> > and parallel_apply_set_idle to parallel_apply_set_xact_finish. We can
> > change the name of the in_remote_transaction flag to in_use.
>
> Agreed. One thing I found when addressing this is that there could be a race
> condition if we want to set the flag in parallel apply worker:
>
> where the leader has already started waiting for the parallel apply worker to
> finish processing the transaction(set the in_parallel_apply_xact to false) while the
> child process has not yet processed the first STREAM_START and has not set the
> in_parallel_apply_xact to true.

Sorry, I didn’t complete this sentence. I meant it's safer to set this flag in apply leader,
So I changed the code like that and added some comments to explain the same.

...
>
> Attach the new version patch set which addressed above comments and also
> fixed another problem while subscriber to a low version publisher.

Attach the correct patch set this time.

Best regards,
Hou zj

Attachment Content-Type Size
v28-0005-Add-a-main_worker_pid-to-pg_stat_subscription.patch application/octet-stream 7.7 KB
v28-0001-Perform-streaming-logical-transactions-by-parall.patch application/octet-stream 128.7 KB
v28-0002-Test-streaming-parallel-option-in-tap-test.patch application/octet-stream 74.5 KB
v28-0003-Add-some-checks-before-using-parallel-apply-work.patch application/octet-stream 49.7 KB
v28-0004-Retry-to-apply-streaming-xact-only-in-apply-work.patch application/octet-stream 60.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kazutaka Onishi 2022-09-05 13:32:19 Re: Asynchronous execution support for Custom Scan
Previous Message Justin Pryzby 2022-09-05 13:02:35 Re: Different compression methods for FPI