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

On Wed, Jan 4, 2023 at 13:31 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Tue, Jan 3, 2023 at 2:40 PM wangw(dot)fnst(at)fujitsu(dot)com
> <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Mon, Jan 2, 2023 at 18:54 PM Amit Kapila
> > <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > > On Fri, Dec 30, 2022 at 3:55 PM wangw(dot)fnst(at)fujitsu(dot)com
> > > <wangw(dot)fnst(at)fujitsu(dot)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:

Thanks for your comments!

> +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.

Agree. Removed this check.

> ---
> +/*
> + * 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'.

Agree. Removed this parameter.

Attach the new patch set.
Apart from addressing Sawada-San's comments, I also did some other minor
changes in the patch:

* Adjusted a testcase about crash restart in 023_twophase_stream.pl, I
skipped the check for DEBUG msg as the msg might not be output if the crash happens
before that.
* Adjusted the code in pg_lock_status() to make the fields of
applytransaction lock display in more appropriate places.
* Add a comment to explain why we unlock the transaction before aborting the
transaction in parallel apply worker.

Best regards,
Hou zj