On Fri, Jul 1, 2022 at 7:12 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Mar 16, 2022 at 11:34 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > While looking at this issue again, I realized there seems to be two > problems with subscription stats on shmem stats: > > Firstly, we call pgstat_create_subscription() when creating a > subscription but the subscription stats are reported by apply workers. > And pgstat_create_subscription() just calls > pgstat_create_transactional(): > > void > pgstat_create_subscription(Oid subid) > { > pgstat_create_transactional(PGSTAT_KIND_SUBSCRIPTION, > InvalidOid, subid); > } > > I guess calling pgstat_create_subscription() is not necessary for the > current usage. On the other hand, if we create the subscription stats > there we can resolve the issue Melanie reported in this thread. >
It won't create the stats entry in the shared hash table, so the behavior should be the same as without shared stats. I am not sure we need to do anything for this one. > 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. -- With Regards, Amit Kapila.