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

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Vitaly Davydov <v.davydov@postgrespro.ru>, pgsql-hackers <pgsql-hackers@postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, tomas@vondra.me
Date: 2025-06-06T05:19:13Z
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, Jun 3, 2025 at 6:51 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> >
> > As per my understanding, for logical slots, effective_xmin is only set
> > during the initial copy phase (or say if one has to export a
> > snapshot), after that, its value won't change. Please read the
> > comments in CreateInitDecodingContext() where we set its value. If you
> > still have questions about it, we can discuss further.
>
> OK, thank you for the clarification.   I've read the comments in
> CreateInitDecodingContext() as you suggested.  All of above makes me
> think *_xmin fields are handled properly.
>

Yes, they handled properly for logical slots, but there is no similar
safety mechanism for physical slots.

One minor comment:
+
+ /* Latest restart_lsn that has been flushed to disk. For persistent slots
+ * the flushed LSN should be taken into account when calculating the oldest

This doesn't follow our practice for multi-line comments.

-- 
With Regards,
Amit Kapila.