On Wed, Mar 6, 2024 at 2:36 PM Bertrand Drouvot <bertranddrouvot...@gmail.com> wrote: > > Hi, > > On Wed, Mar 06, 2024 at 10:24:46AM +0530, shveta malik wrote: > > On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot > > <bertranddrouvot...@gmail.com> wrote: > > 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); > > > > Yeah, it's easier to read and probably reduce the pgstat_replslot.o object > file > size a bit for non optimized build. > > > Something similar in pgstat_fetch_replslot() perhaps? > > Yeah, all of the above done in v3 attached. >
Thanks for the patch. For the fix in pgstat_fetch_replslot(), even with the lock in fetch call, there are chances that the concerned slot can be dropped and recreated. --It can happen in a small window in pg_stat_get_replication_slot() when we are consuming the return values of pgstat_fetch_replslot (using slotent). --Also it can happen at a later stage when we have switched to fetching the next slot (obtained from 'pg_replication_slots' through view' pg_stat_replication_slots'), the previous one can be dropped. Ultimately the results of system view 'pg_replication_slots' can still give us non-existing or re-created slots. But yes, I do not deny that it gives us better consistency protection. Do you feel that the lock in pgstat_fetch_replslot() should be moved to its caller when we are done copying the results of that slot? This will improve the protection slightly. thanks Shveta