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

Vitaly Davydov <v.davydov@postgrespro.ru>

From: "Vitaly Davydov" <v.davydov@postgrespro.ru>
To: pgsql-hackers@lists.postgresql.org
Date: 2024-12-13T14:34:01Z
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().

Attachments

> On 11/21/24 14:59, Tomas Vondra wrote:
>
> I don't have a great idea how to improve this. It seems wrong for
> ReplicationSlotsComputeRequiredLSN() to calculate the LSN using values
> from dirty slots, so maybe it should simply retry if any slot is dirty?
> Or retry on that one slot? But various places update the restart_lsn
> before marking the slot as dirty, so right now this won't work.

To ping the topic, I would like to propose a new version of my patch. All the check-world tests seems to pass ok. 

The idea of the patch is pretty simple - keep flushed restart_lsn in memory and use this value to calculate required lsn in ReplicationSlotsComputeRequiredLSN().

One note - if restart_lsn_flushed is invalid, the restart_lsn value will be used. If we take invalid restart_lsn_flushed instead of valid restart_lsn the slot will be skipped. At the moment I have no other ideas how to deal with invalid restart_lsn_flushed.

With best regards,
Vitaly