On Wednesday, March 9, 2022 3:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Mar 9, 2022 at 11:22 AM Masahiko Sawada
> <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Mar 9, 2022 at 12:37 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> > >
> > > On Wed, Mar 9, 2022 at 6:29 AM Masahiko Sawada
> <sawada.mshk@gmail.com> wrote:
> > > >
> > > > ---
> > > > It might have already been discussed but the worker disables the
> > > > subscription on an error but doesn't work for a fatal. Is that
> > > > expected or should we handle that too?
> > > >
> > >
> > > I am not too sure about handling FATALs with this feature because
> > > this is mainly to aid in resolving conflicts due to various
> > > constraints. It might be okay to retry in case of FATAL which is
> > > possibly due to some system resource error. OTOH, if we see that it
> > > will be good to disable for a FATAL error as well then I think we
> > > can use PG_ENSURE_ERROR_CLEANUP construct. What do you think?
> >
> > I think that since FATAL raised by logical replication workers (e.g.,
> > terminated by DDL or out of memory etc?) is normally not a repeatable
> > error, it's reasonable to retry in this case.
> >
>
> Yeah, I think we can add a comment in the code for this so that future readers
> know that this has been done deliberately.
OK. I've added some comments in the codes.
The v31 addressed other comments on hackers so far.
(a) brush up the TAP test alignment
(b) fix the place of apply_error_callback_arg.origin_name for table sync worker
(c) modify maybe_reread_subscription to exit, when disable_on_error changes
(d) improve getSubscriptions to combine some branches for v15
Kindly check the attached v31.
Best Regards,
Takamichi Osumi