On Thu, Mar 7, 2024 at 11:12 AM Michael Paquier <mich...@paquier.xyz> wrote: >
> Yeah, it is possible that what you retrieve from > pgstat_fetch_replslot() does not refer exactly to the slot's content > under concurrent activity, but you cannot protect a single scan of > pg_stat_replication_slots as of an effect of its design: > pg_stat_get_replication_slot() has to be called multiple times. The > patch at least makes sure that the copy of the slot's stats retrieved > by pgstat_fetch_entry() is slightly more consistent Right, I understand that. , but you cannot do > better than that except if the data retrieved from > pg_replication_slots and its stats are fetched in the same context > function call, holding the replslot LWLock for the whole scan > duration. Yes, agreed. > > > 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. > > I don't see what extra protection this would offer, as > pg_stat_get_replication_slot() is called once for each slot. Feel > free to correct me if I'm missing something. It slightly improves the chances. pgstat_fetch_replslot is only called from pg_stat_get_replication_slot(), I thought it might be better to acquire lock before we call pgstat_fetch_replslot and release once we are done copying the results for that particular slot. But I also understand that it will not prevent someone from dropping that slot at a later stage when the rest of the calls of pg_stat_get_replication_slot() are still pending. So I am okay with the current one. thanks Shveta
