Re: Persist slot invalidations before publishing them

Hi,

On Thu, Sep 24, 2026 at 10:55:03AM +0530, shveta malik wrote:
> 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.

Yeah, makes sense. I changed 0001 to use PG_TRY/PG_CATCH and moved the common
cleanup into ReplicationSlotPersistInvalidation(), with update_inactive_since
passed by the caller.

The I/O lock is still acquired by each caller because 0001 must hold it before
claiming the inactive slot to serialize concurrent internal invalidators.

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

The current ordering should be safe because the invalidation has already been
published, so a concurrent invalidator exits before considering active_proc.

That said, I agree that releasing the slot first means this ordering no longer
relies on that check and makes the success and error paths consistent. So, done
in the attached.

It also adds the check you suggested for the synchronized slot's shared memory
state after a successful synchronization.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com