Re: Missing LWLock protection in pgstat_reset_replslot()

On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)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