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-05-24T11:14:49Z
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 Sat, May 24, 2025 at 4:08 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> I spend more time on this.  The next revision is attached.  It
> contains revised comments and other cosmetic changes.  I'm going to
> backpatch 0001 to all supported branches,
>

Is my understanding correct that we need 0001 because
PhysicalConfirmReceivedLocation() doesn't save the slot to disk after
changing the slot's restart_lsn? If so, shouldn't the comments (One
could argue that the slot should be saved to disk now, but that'd be
energy wasted - the worst thing lost information could cause here is
to give wrong information in a statistics view) in
PhysicalConfirmReceivedLocation() be changed to mention the risk of
not saving the slot?

Also, after 0001, even the same solution will be true for logical
slots as well, where we are already careful to save the slot to disk
if its restart_lsn is changed, see LogicalConfirmReceivedLocation().
So, won't that effort be wasted? Even if we don't want to do anything
about it (which doesn't sound like a good idea), we should note that
in comments somewhere.

-- 
With Regards,
Amit Kapila.