Re: Issue with pg_stat_subscription_stats

Hi,

On 2022-07-01 16:08:48 +0900, Masahiko Sawada wrote:
> Yes, my point is that it may be misleading that the subscription stats
> are created when a subscription is created.

I think it's important to create stats at that time, because otherwise it's
basically impossible to ensure that stats are dropped when a transaction rolls
back. If some / all columns should return something else before stats are
reported that can be addressed easily by tracking that in a separate field.


> On the other hand, I'm not sure we agreed that the behavior that
> Melanie reported is not a problem. The user might get confused since
> the subscription stats works differently than other stats when a
> reset. Previously, the primary reason why I hesitated to create the
> subscription stats when creating a subscription is that CREATE
> SUBSCRIPTION (with create_slot = false) can be rolled back. But with
> the shmem stats, we can easily resolve it by using
> pgstat_create_transactional().

Yep.


> > > The second problem is that the following code in DropSubscription()
> > > should be updated:
> > >
> > >     /*
> > >      * Tell the cumulative stats system that the subscription is getting
> > >      * dropped. We can safely report dropping the subscription statistics here
> > >      * if the subscription is associated with a replication slot since we
> > >      * cannot run DROP SUBSCRIPTION inside a transaction block.  Subscription
> > >      * statistics will be removed later by (auto)vacuum either if it's not
> > >      * associated with a replication slot or if the message for dropping the
> > >      * subscription gets lost.
> > >      */
> > >     if (slotname)
> > >         pgstat_drop_subscription(subid);
> > >
> > > I think we can call pgstat_drop_subscription() even if slotname is
> > > NULL and need to update the comment.
> > >
> >
> > +1.
> >
> > > IIUC autovacuum is no longer
> > > responsible for garbage collection.
> > >
> >
> > Right, this is my understanding as well.
> 
> Thank you for the confirmation.

Want to propose a patch?

Greetings,

Andres Freund