On Wed, Sep 23, 2026 at 2:08 PM Bertrand Drouvot <[email protected]> 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 > > <[email protected]> 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
0001-helper-function.patch
Description: Binary data
