On 01/03/2024 12:15, Bertrand Drouvot wrote:
Hi hackers,

I think that pgstat_reset_replslot() is missing LWLock protection. Indeed, we
don't have any guarantee that the slot is active (then preventing it to be
dropped/recreated) when this function is executed.

Yes, so it seems at quick glance. We have a similar issue in pgstat_fetch_replslot(); it might return stats for wrong slot, if the slot is dropped/recreated concurrently. Do we care?

--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -46,6 +46,8 @@ pgstat_reset_replslot(const char *name)
Assert(name != NULL); + LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
+
        /* Check if the slot exits with the given name. */
        slot = SearchNamedReplicationSlot(name, true);

SearchNamedReplicationSlot() will also acquire the lock in LW_SHARED mode, when you pass need_lock=true. So that at least should be changed to false.

--
Heikki Linnakangas
Neon (https://neon.tech)



Reply via email to