Re: Issue with pg_stat_subscription_stats

On Sat, Jul 2, 2022 at 2:53 Andres Freund <andres(at)anarazel(dot)de> wrote:

> 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?

Yes, I’ll propose a patch.

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