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).
Wouldyou 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.
--
With Regards,
Amit Kapila.
