I had a look at patch001 as well. I have 2 questions: 1) Would it be better to use a PG_TRY/PG_CATCH block in patch 001, similar to patch 002? Currently, the slot is released via PG_ENSURE_ERROR_CLEANUP, while we rely on top-level error cleanup for lock-release. Using TRY/CATCH would let us explicitly release both the slot and I/O lock together, making the error handling consistent across both patches. We could even reuse persist_slot_invalidation() with a small change to pass update_inactive_since from the caller.
2) + /* Let caller know */ + invalidated = true; + LWLockRelease(&s->io_in_progress_lock); ReplicationSlotRelease(); Wouldn't it be better (and safer) to release the slot before releasing the I/O lock? Currently, concurrent invalidators are protected by the 'invalidation_cause == RS_INVAL_NONE' check after acquiring the lock. But releasing the slot first would close this race window entirely. It would also make the order consistent with Patch 002 and the error-handling flow in Patch 001 itself. thanks Shveta
