RE: Optionally automatically disable logical replication subscriptions on error

On Friday, February 25, 2022 12:57 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Thu, Feb 24, 2022 at 6:30 PM Masahiko Sawada
> <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Thu, Feb 24, 2022 at 8:08 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > >
> > > On Thu, Feb 24, 2022 at 1:20 PM Masahiko Sawada
> <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > >
> > > > Here are some comments:
> > > >
> > > > Why do we need SyncTableStartWrapper() and ApplyLoopWrapper()?
> > > >
> > >
> > > I have given this comment to move the related code to separate
> > > functions to slightly simplify ApplyWorkerMain() code but if you
> > > don't like we can move it back. I am not sure I like the new
> > > function names in the patch though.
> >
> > Okay, I'm fine with moving this code but perhaps we can find a better
> > function name as "Wrapper" seems slightly odd to me.
> >
>
> Agreed.
>
> > For example,
> > start_table_sync_start() and start_apply_changes() or something (it
> > seems we use the snake case for static functions in worker.c).
> >
>
> I am fine with something on these lines, how about start_table_sync() and
> start_apply() respectively?
Adopted. (If we come up with better names, we can change those then)

Kindly have a look at attached the v22.
It has incorporated other improvements of TAP test,
refinement of the DisableSubscriptionOnError function and so on.

Best Regards,
Takamichi Osumi