Re: Issue with pg_stat_subscription_stats

On Mon, Mar 14, 2022 at 2:05 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Sat, Mar 12, 2022 at 3:15 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > Hi,
> >
> > On 2022-03-12 08:28:35 +0530, Amit Kapila wrote:
> > > On Sat, Mar 12, 2022 at 2:14 AM Melanie Plageman
> > > <melanieplageman(at)gmail(dot)com> wrote:
> > > >
> > > > So, I noticed that pg_stat_reset_subscription_stats() wasn't working
> > > > properly, and, upon further investigation, I'm not sure the view
> > > > pg_stat_subscription_stats is being properly populated.
> > > >
> > >
> > > I have tried the below scenario based on this:
> > > Step:1 Create some data that generates conflicts and lead to apply
> > > failures and then check in the view:
> >
> > I think the problem is present when there was *no* conflict
> > previously. Because nothing populates the stats entry without an error, the
> > reset doesn't have anything to set the stats_reset field in, which then means
> > that the stats_reset field is NULL even though stats have been reset.
>
> Yes, this is what I meant. stats_reset is not initialized and without
> any conflict happening to populate the stats, after resetting the stats,
> the field still does not get populated. I think this is a bit
> unexpected.
>
> psql (15devel)
> Type "help" for help.
>
> mplageman=# select * from pg_stat_subscription_stats ;
> subid | subname | apply_error_count | sync_error_count | stats_reset
> -------+---------+-------------------+------------------+-------------
> 16398 | mysub | 0 | 0 |
> (1 row)
>
> mplageman=# select pg_stat_reset_subscription_stats(16398);
> pg_stat_reset_subscription_stats
> ----------------------------------
>
> (1 row)
>
> mplageman=# select * from pg_stat_subscription_stats ;
> subid | subname | apply_error_count | sync_error_count | stats_reset
> -------+---------+-------------------+------------------+-------------
> 16398 | mysub | 0 | 0 |
> (1 row)
>

Looking at other statistics such as replication slots, shared stats,
and SLRU stats, it makes sense that resetting it populates the stats.
So we need to fix this issue.

However, I think the proposed fix has two problems; it can create an
entry for non-existing subscriptions if the user directly calls
function pg_stat_get_subscription_stats(), and stats_reset value is
not updated in the stats file as it is not done by the stats
collector.

An alternative solution would be to send the message for creating the
subscription at the end of CRAETE SUBSCRIPTION which basically
resolves them. A caveat is that if CREATE SUBSCRIPTION (that doesn't
involve replication slot creation) is rolled back, the first problem
still occurs. But it should not practically matter as a similar thing
is possible via existing table-related functions for dropped tables.
Also, we normally don't know the OID of subscription that is rolled
back. I've attached a patch for that.

Regards,

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