Re: Issue with pg_stat_subscription_stats

On Fri, Jul 1, 2022 at 7:12 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Wed, Mar 16, 2022 at 11:34 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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.