public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: [email protected] <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
Date: Fri, 21 Oct 2022 09:31:42 +0000
Message-ID: <OS0PR01MB571673E3EE2A65A3B8B7713B942D9@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB586674C1EE91C06DBACE7728F52B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <OS0PR01MB5716730C06159452335D870D947F9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1Lu-6oXMk7ZaGYLwm3CRLBuzueGbasyHnNpJxu6Mq3mmg@mail.gmail.com>
<OS3PR01MB6275F145878B4A44586C46CE9E499@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1LMVdS6uM7Tw7ANL0BetAd76TKkmAXNNQa0haTe2tax6g@mail.gmail.com>
<OS3PR01MB6275EFC4B707650DAB9392859E4D9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAHut+PuVjRgGr4saN7qwq0oB8DANHVR7UfDiciB1Q3cYN54F6A@mail.gmail.com>
<OS3PR01MB627567CF4B96A23DAFAF77269E4E9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1KEf6w1azJytYnLhd2Lo-=rPkRh1Jv2ePcvs=SKqNQnAQ@mail.gmail.com>
<CAA4eK1KjGNA8T8O77rRhkv6bRT6OsdQaEy--2hNrJFCc80bN0A@mail.gmail.com>
<OS0PR01MB571620741B90C769D2727D7F94559@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAHut+PvFENKb5fcMko5HHtNEAaZyNwGhu3PASrcBt+HFoFL=Fw@mail.gmail.com>
<OS0PR01MB5716B400CD81565E868616DB945F9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS3PR01MB6275328AD1B56164C66A552D9E229@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAA4eK1Jp4xEC_hJvanrcroo-DGX8CMjZw_D4SqZOe7Az+xrPQA@mail.gmail.com>
<OS0PR01MB57160760B34E1655718F4D1994249@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAHut+PsY0aevdVqeCUJOrRQMrwpg5Wz3-Mo+bU=mCxW2+9EBTg@mail.gmail.com>
<OS0PR01MB57160B0C0FDDCED638639696942B9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<TYAPR01MB586674C1EE91C06DBACE7728F52B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
On Wednesday, October 19, 2022 8:50 PM Kuroda, Hayato/黒田 隼人 <[email protected]> wrote:
>
> ===
> 01. applyparallelworker.c - SIZE_STATS_MESSAGE
>
> ```
> /*
> * There are three fields in each message received by the parallel apply
> * worker: start_lsn, end_lsn and send_time. Because we have updated these
> * statistics in the leader apply worker, we can ignore these fields in the
> * parallel apply worker (see function LogicalRepApplyLoop).
> */
> #define SIZE_STATS_MESSAGE (2 * sizeof(XLogRecPtr) + sizeof(TimestampTz))
> ```
>
> According to other comment styles, it seems that the first sentence of the
> comment should represent the datatype and usage, not the detailed reason.
> For example, about ParallelApplyWorkersList, you said "A list ...". How about
> adding like following message:
> The message size that can be skipped by parallel apply worker
Thanks for the comments, but the current description seems enough to me.
> ~~~
> 02. applyparallelworker.c - parallel_apply_start_subtrans
>
> ```
> if (current_xid != top_xid &&
> !list_member_xid(subxactlist, current_xid)) ```
>
> A macro TransactionIdEquals is defined in access/transam.h. Should we use it,
> or is it too trivial?
I checked the existing codes, it seems both style are being used.
Maybe we can post a separate patch to replace them later.
> ~~~
> 08. worker.c - apply_handle_prepare_internal
>
> Same as above.
>
>
> ~~~
> 09. worker.c - maybe_reread_subscription
>
> ```
> /*
> * Exit if any parameter that affects the remote connection was
> changed.
> * The launcher will start a new worker.
> */
> if (strcmp(newsub->conninfo, MySubscription->conninfo) != 0 ||
> strcmp(newsub->name, MySubscription->name) != 0 ||
> strcmp(newsub->slotname, MySubscription->slotname) != 0 ||
> newsub->binary != MySubscription->binary ||
> newsub->stream != MySubscription->stream ||
> strcmp(newsub->origin, MySubscription->origin) != 0 ||
> newsub->owner != MySubscription->owner ||
> !equal(newsub->publications, MySubscription->publications))
> {
> ereport(LOG,
> (errmsg("logical replication apply worker for
> subscription \"%s\" will restart because of a parameter change",
> MySubscription->name)));
>
> proc_exit(0);
> }
> ```
>
> When the parallel apply worker has been launched and then the subscription
> option has been modified, the same message will appear twice.
> But if the option "streaming" is changed from "parallel" to "on", one of them
> will not restart again.
> Should we modify message?
Thanks, it seems a timing problem, if the leader catch the change first and stop
the parallel workers, the message will only appear once. But I agree we'd
better make the message clear. I changed the message in parallel apply worker.
While on it, I also adjusted some other message to include "parallel apply
worker" if they are in parallel apply worker.
Best regards,
Hou zj
view thread (625+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
In-Reply-To: <OS0PR01MB571673E3EE2A65A3B8B7713B942D9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox