On Sat, Jul 2, 2022 at 2:53 Andres Freund <and...@anarazel.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/
