Re: Missing LWLock protection in pgstat_reset_replslot()

On Thu, Mar 07, 2024 at 10:57:28AM +0530, shveta malik wrote:
> --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).

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, 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.

> 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.
--
Michael