Horton

Re: Newly created replication slot may be invalidated by checkpoint

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Vitaly Davydov <v.davydov@postgrespro.ru>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "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>
Date: 2025-11-13T04:18:57Z
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

On Thu, Nov 13, 2025 at 8:03 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Wednesday, November 12, 2025 11:55 PMVitaly Davydov <v.davydov@postgrespro.ru> wrote:
> >
> > On Monday, November 10, 2025 12:11 MSK, Amit Kapila
> > <amit.kapila16@gmail.com> wrote:
> > > The fix seems to be only provided for bank branches, but IIUC the
> > > problem can happen in HEAD as well. In Head, how about acquiring
> > > ReplicationSlotAllocationLock in Exclusive mode during
> > > ReplicationSlotReserveWal? This lock is acquired in SHARE mode in
> > > CheckPointReplicationSlots. So, this should make our calculations
> > > correct and avoid invalidating the newly created slot.
> >
> > Amit, I'm not sure how it may help to avoid the change of redo rec ptr during
> > wal reservation by a slot, because it doesn't serialize redo rec ptr assignment
> > and slot's wal reservation, but the core issue is in reco rec ptr change when we
> > reserve the wal by a slot.
>
> I think the main issue here lies in the possibility that the minimum restart_lsn
> obtained during a checkpoint could be less than the WAL position that is being
> reserved concurrently. So, instead of serializing the redo assignment and WAL
> reservation, Amit proposes serializing the CheckPointReplicationSlots() and WAL
> reservation. This would ensure the following:
>
> 1) If the WAL reservation occurs first, the checkpoint must wait for the
> restart_lsn to be updated before proceeding with WAL removal. This guarantees
> that the most recent restart_lsn position is detected.
>
> 2) If the checkpoint calls CheckPointReplicationSlots() first, then any
> subsequent WAL reservation must take a position later than the redo pointer.
>

Your understanding is correct. This whole difference of the way same
problem can happen in HEAD and back branches but in different ways
because the code varies a lot between branches. Now, if we again put a
different fix for the newly discovered problem, then it will take the
code much further away adding more to the maintenance burden. So, I
thought we should once discuss making the code same in HEAD and back
branches. Can we do some research to see if any of the available
extensions uses sizeof(ReplicationSlot)? I think even if we don't find
any such extension there will always be a chance that there is some
closed source extension that might rely on it but I think still this
is worth considering to change ABI in back branches for this fix. This
code area is quite subtle where the bugs exist from a long time and
the initial fix also didn't close all holes.

-- 
With Regards,
Amit Kapila.