On Wednesday, November 19, 2025 4:24 PM Hou, Zhijie<[email protected]> wrote: > > On Monday, November 17, 2025 6:50 PM Zhijie Hou (Fujitsu) > <[email protected]> wrote: > > > > > > Attach v2 patch that improves the fix based on above analysis. > > > > (I am still testing some other scenarios related to slotsync to ensure the > > current fix is complete) > > I have been testing whether taking exclusive ReplicationSlotAllocationLock > can > prevent newly synced slot from being invalidated, but found that it is > insufficient. > > During slotsync, since it fetches the remote LSN (queried from the publisher) > as > the initial restart_lsn for newly synced slot and we sync the slots in a > asynchronous manner, the synced restart_lsn could be behind the standby's > redo > pointer. So, there could be race conditions that the checkpoint removes the > required WALs and invalidates the newly synced slot: > > 1. Assuming there is no slot on standby, the minimum slot LSN would be > invalid, > and the checkpoint reaches InvalidateObsoleteReplicationSlots() with > oldestSegno =segno of redo. > 2. The slotsync successfully sync the slot A that has old restart_lsn. > 3. The checkpoint would then find that the newly synced slot A should be > invalidated due to old restart_lsn. .. > > Here is the V3 patch that fixes all the race conditions found so far.
I am attaching a tap-test in 0002 that includes an injection point to reproduce this scenario. The test expects that while syncing a slot to the standby server, if the WAL before the remote restart_lsn is at risk of being removed by a checkpoint, the slot cannot be synced. Without the fix in 0001, the test would fail because the slot is synced to standby but immediately invalidated by the checkpoint. I am not adamant about merging this test but just for reference. Besides, I fixed a bug in v3-0001 where the restart_lsn is set to a wrong value. > > ------ > > Apart from addressing the fix for HEAD, I would like to inquire if anyone > holds > a differing opinion regarding the revert of the origin fix 2090edc6f32f652a2c > in > the back branches and applying the same fix as HEAD. > > I searched for extensions that rely on the size of ReplicationSlot and did not > find any, so I think it is OK to implement the same fix in the backpatches. > > It there are no alternative views, I will proceed with preparing the patches > for > the back branches in upcoming versions. Best Regards, Hou zj
v4-0002-Add-a-tap-test-using-injection-point.patch
Description: v4-0002-Add-a-tap-test-using-injection-point.patch
v4-0001-Fix-race-conditions-causing-invalidation-of-newly.patch
Description: v4-0001-Fix-race-conditions-causing-invalidation-of-newly.patch
