Horton

RE: Newly created replication slot may be invalidated by checkpoint

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: suyu.cmj <mengjuan.cmj@alibaba-inc.com>, tomas <tomas@vondra.me>, michael <michael@paquier.xyz>, bharath.rupireddyforpostgres <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Alexander Korotkov <aekorotkov@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Vitaly Davydov <v.davydov@postgrespro.ru>
Date: 2025-11-19T11:40:31Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Prevent invalidation of newly synced replication slots.

  2. Prevent invalidation of newly created replication slots.

  3. Fix re-distributing previously distributed invalidation messages during logical decoding.

  4. Keep WAL segments by the flushed value of the slot's restart LSN

  5. Keep WAL segments by slot's last saved restart LSN

Attachments

On Wednesday, November 19, 2025 4:24 PM Hou, Zhijie<houzj.fnst@fujitsu.com> wrote:
> 
> On Monday, November 17, 2025 6:50 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> 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