Re: Optionally automatically disable logical replication subscriptions on error

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

--
With Regards,
Amit Kapila.