Re: Missing LWLock protection in pgstat_reset_replslot()

On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> > /*
> > * Nothing to do for physical slots as we collect stats only for logical
> > * slots.
> > */
> > if (SlotIsPhysical(slot))
> > return;
>
> D'oh! Thanks! Fixed in v2 shared up-thread.

Thanks.  Can we try to get rid of multiple LwLockRelease in
pgstat_reset_replslot(). Is this any better?

        /*
-        * Nothing to do for physical slots as we collect stats only for logical
-        * slots.
+        * Reset stats if it is a logical slot. Nothing to do for physical slots
+        * as we collect stats only for logical slots.
         */
-       if (SlotIsPhysical(slot))
-       {
-               LWLockRelease(ReplicationSlotControlLock);
-               return;
-       }
-
-       /* reset this one entry */
-       pgstat_reset(PGSTAT_KIND_REPLSLOT, InvalidOid,
-                                ReplicationSlotIndex(slot));
+       if (SlotIsLogical(slot))
+               pgstat_reset(PGSTAT_KIND_REPLSLOT, InvalidOid,
+                                        ReplicationSlotIndex(slot));

        LWLockRelease(ReplicationSlotControlLock);


Something similar in pgstat_fetch_replslot() perhaps?

thanks
Shveta