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: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Vitaly Davydov <v.davydov@postgrespro.ru>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-04-29T01:39:23Z
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 Tue, Apr 29, 2025 at 4:03 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Apr 28, 2025 at 8:17 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > > I have a question - is there any interest to backport the solution into
> > > existing major releases?
> >
> > As long as this is the bug, it should be backpatched to all supported
> > affected releases.
>
> Yes, but I think we cannot back-patch the proposed fix to back
> branches as it changes the ReplicationSlot struct defined in slot.h,
> which breaks ABI compatibility.

Yes, and I think Vitaly already proposed to address this issue.  This
aspect also needs to be carefully reviewed for sure.

On Thu, Apr 24, 2025 at 5:32 PM Vitaly Davydov <v.davydov@postgrespro.ru> wrote:
> I can prepare a patch where restart_lsn_flushed stored
> outside of ReplicationSlot structure and doesn't affect the existing API.

------
Regards,
Alexander Korotkov
Supabase