On Mon, Sep 27, 2021 at 12:46 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Sep 24, 2021 at 7:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Sep 3, 2021 at 4:33 AM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
> >
> > > I am attaching a version of such a function, plus some tests of your patch (since it does not appear to have
any). Would you mind reviewing these and giving comments or including them in your next patch version?
> > >
> >
> > I've looked at the patch and here are some comments:
> >
> > +
> > +-- no errors should be reported
> > +SELECT * FROM pg_stat_subscription_errors;
> > +
> >
> > +
> > +-- Test that the subscription errors view exists, and has the right columns
> > +-- If we expected any rows to exist, we would need to filter out unstable
> > +-- columns. But since there should be no errors, we just select them all.
> > +select * from pg_stat_subscription_errors;
> >
> > The patch adds checks of pg_stat_subscription_errors in order to test
> > if the subscription doesn't have any error. But since the subscription
> > errors are updated in an asynchronous manner, we cannot say the
> > subscription is working fine by checking the view only once.
> >
>
> One question I have here is, can we reliably write few tests just for
> the new view patch? Right now, it has no test, having a few tests will
> be better. Here, because the apply worker will keep on failing till we
> stop it or resolve the conflict, can we rely on that fact? The idea
> is that even if one of the entry is missed by stats collector, a new
> one (probably the same one) will be issued and we can wait till we see
> one error in view. We can add additional PostgresNode.pm
> infrastructure once the main patch is committed.
Yes, the new tests added by 0003 patch (skip_xid patch) use that fact.
After the error is shown in the view, we fetch the XID from the view
to specify as skip_xid. The tests just for the
pg_stat_subscription_errors view will be a subset of these tests. So
probably we can add it in 0001 patch and 0003 patch can extend the
tests so that it tests skip_xid option.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/