Re: Persist slot invalidations before publishing them

On Wed, Sep 23, 2026 at 2:08 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Sep 23, 2026 at 12:00:39PM +0530, shveta malik wrote:
> > On Wed, Sep 23, 2026 at 11:37 AM Bertrand Drouvot
> > <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> > >
> > > Another possibility would be to make ReplicationSlotPersistInvalidation() always
> > > leave the caller acquired I/O lock held. Slotsync could then release that specific
> > > lock in a PG_CATCH() block, something like:
> >
> > Yes, I agree. I find this approach much better for 2 reasons:
> >
> > 1) The caller has better control over the lock, which makes sense
> > since it is the one acquiring it.
> > 2) It makes the code much more understandable. Earlier, it took me a
> > while to figure out exactly where the io_in_progress_lock was getting
> > released, especially looking at the slotsync patch where it was
> > acquired right before calling ReplicationSlotPersistInvalidation().
> >
> > > "
> > > PG_CATCH();
> > > {
> > > HOLD_INTERRUPTS();
> > > LWLockRelease(&slot->io_in_progress_lock);
> > > PG_RE_THROW();
> > > }
> > > PG_END_TRY();
> > >
> > > LWLockRelease(&slot->io_in_progress_lock);
> > > "
> > >
> > > This would avoid both LWLockReleaseAll() and using LWLockHeldByMe() for normal
> > > control flow. Does that sound preferable?
> >
> > Yes.
>
> Thanks! Done that way in v4 attached.
>

Can we create the helper function for this logic as it is not core to
the sync-function. Please use the attached patch if you agree, and
feel free to change comments as you see apt.

This approach is better for one more reason: v3 was only releasing the
I/O lock locally during the API call (via slotsync_failure_callback),
leaving the slot-sync worker to rely on top-level proc_exit perhaps.
Current approach ensures consistent lock lifecycle handling for both
the worker and the API.

thanks
Shhveta