RE: Optionally automatically disable logical replication subscriptions on error

On Monday, March 7, 2022 5:45 PM Amit Kaila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Mar 7, 2022 at 4:55 AM Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote:
> >
> > On Fri, Mar 4, 2022 at 5:55 PM Masahiko Sawada
> <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > ---
> > > + /*
> > > + * First, ensure that we log the error message so
> > > that it won't be
> > > + * lost if some other internal error occurs in the
> > > following code.
> > > + * Then, abort the current transaction and send the
> > > stats message of
> > > + * the table synchronization failure in an idle state.
> > > + */
> > > + HOLD_INTERRUPTS();
> > > + EmitErrorReport();
> > > + FlushErrorState();
> > > + AbortOutOfAnyTransaction();
> > > + RESUME_INTERRUPTS();
> > > +
> > > + pgstat_report_subscription_error(MySubscription->oid, false);
> > > +
> > > + if (MySubscription->disableonerr)
> > > + {
> > > + DisableSubscriptionOnError();
> > > + proc_exit(0);
> > > + }
> > > +
> > > + PG_RE_THROW();
> > >
> > > If the disableonerr is false, the same error is reported twice.
> > > Also, the code in PG_CATCH() in both start_apply() and
> > > start_table_sync() are almost the same. Can we create a common
> > > function to do post-error processing?
> > >
> > > The worker should exit with return code 1.
> > >
> > > I've attached a fixup patch for changes to worker.c for your
> > > reference. Feel free to adopt the changes.
> >
> > The way that common function is implemented required removal of the
> > existing PG_RE_THROW logic, which in turn was only possible using
> > special knowledge that this just happens to be the last try/catch
> > block for the apply worker.
> >
>
> I think we should re_throw the error in case we have not handled it by disabling
> the subscription (in which case we can exit with success code (0)).
Agreed. Fixed the patch so that it use re_throw.

Another point I changed from v28 is the order
to call AbortOutOfAnyTransaction and FlushErrorState,
which now is more aligned with other places.

Kindly check the attached v29.

Best Regards,
Takamichi Osumi