Re: Optionally automatically disable logical replication subscriptions on error

On Mon, Jun 21, 2021 at 7:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 11:19 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Jun 21, 2021 at 10:55 AM Mark Dilger
> > <mark.dilger@enterprisedb.com> wrote:
> >
> > >  I don't mind if you want to store more information, and maybe that needs to be stored somewhere else.  Do you
believepg_subscription_rel is a suitable location?
 
> > >
> > It won't be sufficient to store information in either
> > pg_subscription_rel or pg_susbscription. I think if we want to store
> > the required information in a catalog then we need to define a new
> > catalog (pg_subscription_conflicts or something like that) with
> > information corresponding to each rel in subscription (srsubid oid
> > (Reference to subscription), srrelid oid (Reference to relation),
> > <columns for error_info>). OTOH, we can choose to send the error
> > information to stats collector which will then be available via stat
> > view and update system catalog to disable the subscription but there
> > will be a risk that we might send info of failed transaction to stats
> > collector but then fail to update system catalog to disable the
> > subscription.
> >
>
> I think we should store the input from the user (like disable_on_error
> flag or xid to skip) in the system catalog pg_subscription and send
> the error information (subscrtion_id, rel_id, xid of failed xact,
> error_code, error_message, etc.) to the stats collector which can be
> used to display such information via a stat view.
>
> The disable_on_error flag handling could be that on error it sends the
> required error info to stats collector and then updates the subenabled
> in pg_subscription. In rare conditions, where we are able to send the
> message but couldn't update the subenabled info in pg_subscription
> either due to some error or server restart, the apply worker would
> again try to apply the same change and would hit the same error again
> which I think should be fine because it will ultimately succeed.
>
> The skip xid handling will also be somewhat similar where on an error,
> we will send the error information to stats collector which will be
> displayed via stats view. Then the user is expected to ask for skip
> xid (Alter Subscription ... SKIP <xid_value>) based on information
> displayed via stat view. Now, the apply worker can skip changes from
> such a transaction, and then during processing of commit record of the
> skipped transaction, it should update xid to invalid value, so that
> next time that shouldn't be used. I think it is important to update
> xid to an invalid value as part of the skipped transaction because
> otherwise, after the restart, we won't be able to decide whether we
> still want to skip the xid stored for a subscription.

Sounds reasonable.

The feature that sends the error information to the stats collector is
a common feature for both and itself is also useful. As discussed in
that skip transaction patch thread, it would also be good if we write
error information (relation, action, xid, etc) to the server log too.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/