Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Vitaly Davydov <v.davydov@postgrespro.ru>, vignesh C <vignesh21@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.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-06-20T02:55:34Z
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 Fri, Jun 20, 2025 at 5:48 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> >
> > If what I said above is correct, then the following part of the commit
> > message will be incorrect:
> > "As stated in the ReplicationSlotReserveWal() comment, this is not
> > always true. Additionally, this issue has been spotted by some
> > buildfarm
> > members."
>
> I agree, this comment needs improvement in terms of clarity.
>
> Meanwhile I've pushed the patch for TAP tests, which I think didn't
> get any objections.
>

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.

-- 
With Regards,
Amit Kapila.