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

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>

To: Vitaly Davydov <v.davydov@postgrespro.ru>

Cc: vignesh C <vignesh21@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Tom Lane <tgl@sss.pgh.pa.us>, Alexander Lakhin <exclusion@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, "tomas@vondra.me" <tomas@vondra.me>

Date: 2025-06-18T16:47:09Z

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 →

Update comment for ReplicationSlot.last_saved_restart_lsn
- 0810fbb02dbe 19 (unreleased) landed
Fix CheckPointReplicationSlots() with max_replication_slots == 0
- 7195c804bd12 18.0 landed
Remove excess assert from InvalidatePossiblyObsoleteSlot()
- 70d8a91f82f1 18.0 landed
Improve runtime and output of tests for replication slots checkpointing.
- 5ed50f9386f0 17.6 landed
- 4464fddf7b50 18.0 landed
Keep WAL segments by slot's last saved restart LSN
- ca307d5cec90 18.0 landed
Fix an assert in CheckPointReplicationSlots().
- d1ffcc7fa3c5 17.0 cited

Attachments

v4-0002-Remove-excess-assert-from-InvalidatePossiblyObsol.patch (application/octet-stream) patch v4-0002
v4-0001-Improve-runtime-and-output-of-tests-for-replicati.patch (application/octet-stream) patch v4-0001

On Wed, Jun 18, 2025 at 6:50 PM Vitaly Davydov <v.davydov@postgrespro.ru> wrote:
> > I think, it is a good idea. Once we do not use the generated data, it is ok
> > just to generate WAL segments using the proposed function. I've tested this
> > function. The tests worked as expected with and without the fix. The attached
> > patch does the change.
>
> Sorry, forgot to attach the patch. It is created on the current master branch.
> It may conflict with your corrections. I hope, it could be useful.

Thank you.  I've integrated this into a patch to improve these tests.

Regarding assertion failure, I've found that assert in
PhysicalConfirmReceivedLocation() conflicts with restart_lsn
previously set by ReplicationSlotReserveWal().  As I can see,
ReplicationSlotReserveWal() just picks fresh XLogCtl->RedoRecPtr lsn.
So, it doesn't seems there is a guarantee that restart_lsn never goes
backward.  The commit in ReplicationSlotReserveWal() even states there
is a "chance that we have to retry".  Thus, I propose to remove the
assertion introduced by ca307d5cec90.

Any objection from backpatching 0001 though 17 and pushing 0002 to the head?

------
Regards,
Alexander Korotkov
Supabase