Horton

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

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Alexander Korotkov' <aekorotkov@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, Vitaly Davydov <v.davydov@postgrespro.ru>, 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>, vignesh C <vignesh21@gmail.com>
Date: 2025-06-27T02:22:27Z
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().

Dear Alexander,

> Regarding last_saved_restart_lsn_updated, I think the opposite.  I
> think we should check if last_saved_restart_lsn_updated is set already
> only if it could promise us some economy of resources.  In our case
> the main check only compares two fields of slot.  And that fields are
> to be accessed anyway.  So, we are not going to save any RAM accesses.
> Therefore, checking for last_saved_restart_lsn_updated seems like
> unnecessary code complication (and I don't see we're doing that in
> other places).  So, I'm going to push this patch "as is".

To clarify: I have no objections. Thanks for giving the knowledge.

Best regards,
Hayato Kuroda
FUJITSU LIMITED