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

On Tue, May 2, 2023 at 9:46 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, May 2, 2023 at 9:06 AM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Friday, April 28, 2023 2:18 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > >
> > > > Alexander, does the proposed patch fix the problem you are facing?
> > > > Sawada-San, and others, do you see any better way to fix it than what
> > > > has been proposed?
> > >
> > > I'm concerned that the idea of relying on IsNormalProcessingMode()
> > > might not be robust since if we change the meaning of
> > > IsNormalProcessingMode() some day it would silently break again. So I
> > > prefer using something like InitializingApplyWorker, or another idea
> > > would be to do cleanup work (e.g., fileset deletion and lock release)
> > > in a separate callback that is registered after connecting to the
> > > database.
> >
> > Thanks for the review. I agree that it’s better to use a new variable here.
> > Attach the patch for the same.
> >
>
> + *
> + * However, if the worker is being initialized, there is no need to release
> + * locks.
> */
> - LockReleaseAll(DEFAULT_LOCKMETHOD, true);
> + if (!InitializingApplyWorker)
> + LockReleaseAll(DEFAULT_LOCKMETHOD, true);
>
> Can we slightly reword this comment as: "The locks will be acquired
> once the worker is initialized."?
>

After making this modification, I pushed your patch. Thanks!

--
With Regards,
Amit Kapila.