MIME-Version: 1.0
References: <CAA4eK1KdVRffYEYiv9=4tVWhP=Jkfh1h6FM0A1R0NEe8cLT8+g@mail.gmail.com>
 <CAA4eK1JU4J3-Z=fcMcyQw-iG-DNr=u6w_g-iM_4xwg1pKupkFQ@mail.gmail.com>
 <OS0PR01MB57163693482C5470C41914AF94EA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAHut+PuYJz-26QWMSRN_HwiJNfYG4ZxEV3RGE-VyZtXXeFm+cw@mail.gmail.com>
 <OS0PR01MB5716C1C1D8244BA86311193894EB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAA4eK1LrADDNXKRVYox4Q+zSFQvtiQu-7piyd_PCmVjg1gsN2w@mail.gmail.com>
 <OS0PR01MB57169EF567017861EACBD10D94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB571623EFD8903C93424EFADC94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com>
 <OS3PR01MB6275B61076717E4CE9E079D19EED9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <CAA4eK1LXch0EDB9RCDiqqmM3S9_cam=svMMK08i8Fmspms=6Xg@mail.gmail.com>
 <OS3PR01MB62756CC6FFBDD431F2878ACE9EF29@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <CAA4eK1Konv5gm2bZhej_CZqotr=qhUHnhhiJixKLRmCMsg+Egw@mail.gmail.com>
 <OS3PR01MB627586DA752A9E07F02FB7609EF09@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <CAA4eK1+63gaq1ruaUTvhM6-GRWi0waZT7X+TJL2tx+dWzisveQ@mail.gmail.com> <OS3PR01MB6275D4B22266412FEEBDB3AE9EF49@OS3PR01MB6275.jpnprd01.prod.outlook.com>
In-Reply-To: <OS3PR01MB6275D4B22266412FEEBDB3AE9EF49@OS3PR01MB6275.jpnprd01.prod.outlook.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Wed, 4 Jan 2023 14:31:25 +0900
Message-ID: <CAD21AoC=PCkeURevhZdNjcKHYws=paPcotgG1fAnq9g8GdwGXw@mail.gmail.com>
Subject: Re: Perform streaming logical transactions by background workers and
 parallel apply
To: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, 
	"houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, 
	Dilip Kumar <dilipbalaut@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAD21AoC%3DPCkeURevhZdNjcKHYws%3DpaPcotgG1fAnq9g8GdwGXw%40mail.gmail.com>
Precedence: bulk

On Tue, Jan 3, 2023 at 2:40 PM wangw.fnst@fujitsu.com
<wangw.fnst@fujitsu.com> wrote:
>
> On Mon, Jan 2, 2023 at 18:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Fri, Dec 30, 2022 at 3:55 PM wangw.fnst@fujitsu.com
> > <wangw.fnst@fujitsu.com> wrote:
> > >
> > > I've checked it and it looks good to me.
> > > Rebased the other patches and ran the pgident for the patch set.
> > >
> > > Attach the new patch set.
> > >
> >
> > I have added a few DEBUG messages and changed a few comments in the
> > 0001 patch. With that v71-0001* looks good to me and I'll commit it
> > later this week (by Thursday or Friday) unless there are any major
> > comments or objections.
>
> Thanks for your improvement.
>
> Rebased the patch set because the new change in HEAD (c8e1ba7).
> Attach the new patch set.

There are some unused parameters in v72 patches:

+static bool
+pa_can_start(TransactionId xid)
+{
+        Assert(TransactionIdIsValid(xid));

'xid' is used only for the assertion check but I don't think it's necessary.

---
+/*
+ * Make sure the leader apply worker tries to read from our error
queue one more
+ * time. This guards against the case where we exit uncleanly without sending
+ * an ErrorResponse, for example because some code calls proc_exit directly.
+ */
+static void
+pa_shutdown(int code, Datum arg)

Similarly, we don't use 'code' here.

---
+/*
+ * Handle a single protocol message received from a single parallel apply
+ * worker.
+ */
+static void
+HandleParallelApplyMessage(ParallelApplyWorkerInfo *winfo, StringInfo msg)

In addition, the same is true for 'winfo'.

The rest looks good to me.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com