Re: Issue with pg_stat_subscription_stats - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: Issue with pg_stat_subscription_stats
Date
Msg-id CAD21AoB3M1k2x-WNUUgBiBvqkGqhv6v-NPkjfnfKz2brGEoU7w@mail.gmail.com
Whole thread Raw
In response to Re: Issue with pg_stat_subscription_stats  (Andres Freund <andres@anarazel.de>)
Responses Re: Issue with pg_stat_subscription_stats
List pgsql-hackers
On Wed, Jul 6, 2022 at 6:52 AM Andres Freund <andres@anarazel.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/

Attachment

pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Using PQexecQuery in pipeline mode produces unexpected Close messages
Next
From: David Rowley
Date:
Subject: Re: tuplesort Generation memory contexts don't play nicely with index builds