Re: Issue with pg_stat_subscription_stats

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