Horton

Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Vitaly Davydov <v.davydov@postgrespro.ru>, vignesh C <vignesh21@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Alexander Lakhin <exclusion@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, "tomas@vondra.me" <tomas@vondra.me>
Date: 2025-07-18T10:44:47Z
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. Update comment for ReplicationSlot.last_saved_restart_lsn

  2. Fix CheckPointReplicationSlots() with max_replication_slots == 0

  3. Remove excess assert from InvalidatePossiblyObsoleteSlot()

  4. Improve runtime and output of tests for replication slots checkpointing.

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

  6. Fix an assert in CheckPointReplicationSlots().

On Sun, Jun 29, 2025 at 9:22 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
> Thanks everyone who are working on the bug. IIUC the remained task is
> to add code comments for avoiding the same mistake again described here:
>
> > Sounds reasonable. As per analysis till now, it seems removal of new
> > assert is correct and we just need to figure out the reason in all
> > failure cases as to why the physical slot's restart_lsn goes backward,
> > and then add a comment somewhere to ensure that we don't repeat a
> > similar mistake in the future.
>
> I've wrote a draft for that. How do you think?

Looks good to me.  I'm going to push this if no objections.

------
Regards,
Alexander Korotkov
Supabase