Re: Issue with pg_stat_subscription_stats

On Wed, Jul 6, 2022 at 6:52 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2022-07-04 11:01:01 +0900, Masahiko Sawada wrote:
> > I've attached the patch, fix_drop_subscriptions_stats.patch, to fix it.
>
> LGTM. Unless somebody sees a reason not to, I'm planning to commit that to 15
> and HEAD.
>
>
> > I've also attached another PoC patch,
> > poc_create_subscription_stats.patch, to create the stats entry when
> > creating the subscription, which address the issue reported in this
> > thread; the pg_stat_reset_subscription_stats() doesn't update the
> > stats_reset if no error is reported yet.
>
> It'd be good for this to include a test.

Agreed.

>
>
> > diff --git a/src/backend/utils/activity/pgstat_subscription.c b/src/backend/utils/activity/pgstat_subscription.c
> > index e1072bd5ba..ef318b7422 100644
> > --- a/src/backend/utils/activity/pgstat_subscription.c
> > +++ b/src/backend/utils/activity/pgstat_subscription.c
> > @@ -47,8 +47,20 @@ pgstat_report_subscription_error(Oid subid, bool is_apply_error)
> > void
> > pgstat_create_subscription(Oid subid)
> > {
> > + PgStat_EntryRef *entry_ref;
> > + PgStatShared_Subscription *shstatent;
> > +
> > pgstat_create_transactional(PGSTAT_KIND_SUBSCRIPTION,
> > InvalidOid, subid);
> > +
> > + entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_SUBSCRIPTION,
> > + InvalidOid, subid,
> > + false);
> > + shstatent = (PgStatShared_Subscription *) entry_ref->shared_stats;
> > +
> > + memset(&shstatent->stats, 0, sizeof(shstatent->stats));
> > +
> > + pgstat_unlock_entry(entry_ref);
> > }
> >
> > /*
>
> I think most of this could just be pgstat_reset_entry().

I think pgstat_reset_entry() doesn't work for this case as it skips
resetting the entry if it doesn't exist.

I've attached an updated patch.

Regards,

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