Re: Optionally automatically disable logical replication subscriptions on error

On Thu, Feb 24, 2022 at 1:20 PM Masahiko Sawada  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.

> ---
> +       /*
> +        * Log the error that caused DisableSubscriptionOnError to be called. We
> +        * do this immediately so that it won't be lost if some other internal
> +        * error occurs in the following code.
> +        */
> +       EmitErrorReport();
> +       AbortOutOfAnyTransaction();
> +       FlushErrorState();
>
> Do we need to hold interrupts during cleanup here?
>

I think so. We do prevent interrupts via
HOLD_INTERRUPTS/RESUME_INTERRUPTS during cleanup.


-- 
With Regards,
Amit Kapila.