Horton

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

Dilip Kumar <dilipbalaut@gmail.com>

View thread
From: Dilip Kumar <dilipbalaut@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@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-25T06:17:46Z
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 Wed, Jun 25, 2025 at 10:57 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Hi,
>
> After commit ca307d5, I noticed another crash when testing
> some other logical replication features.
>
> The server with max_replication_slots set to 0 would crash when executing CHECKPOINT.
>
> TRAP: failed Assert("ReplicationSlotCtl != NULL"), File: "slot.c", Line: 1162, PID: 577315
> postgres: checkpointer (ExceptionalCondition+0x9e)[0xc046cb]
> postgres: checkpointer (ReplicationSlotsComputeRequiredLSN+0x30)[0x99697f]
> postgres: checkpointer (CheckPointReplicationSlots+0x191)[0x997dc1]
> postgres: checkpointer [0x597b1b]
> postgres: checkpointer (CreateCheckPoint+0x6d1)[0x59729e]
> postgres: checkpointer (CheckpointerMain+0x559)[0x93ee79]
> postgres: checkpointer (postmaster_child_launch+0x15f)[0x940311]
> postgres: checkpointer [0x9468b0]
> postgres: checkpointer (PostmasterMain+0x1258)[0x9434f8]
> postgres: checkpointer (main+0x2fe)[0x7f5f9c]
> /lib64/libc.so.6(__libc_start_main+0xe5)[0x7f7585f81d85]
> postgres: checkpointer (_start+0x2e)[0x4958ee]
>
> I think it is trying to access the replication slots when the shared memory
> for them was not allocated.


I do not understand why CheckPointReplicationSlots() calls
ReplicationSlotsComputeRequiredLSN() unconditionally, shouldn't this
be called under the check[1], If not then instead of asserting
Assert("ReplicationSlotCtl != NULL"), this should just return if
ReplicationSlotCtl is NULL, isn't it, because ReplicationSlotCtl is
not allocated if max_replication_slots is 0.


[1]

--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c

@@ -2131,7 +2131,8 @@ CheckPointReplicationSlots(bool is_shutdown)
         * Recompute the required LSN as SaveSlotToPath() updated
         * last_saved_restart_lsn for slots.
         */
-       ReplicationSlotsComputeRequiredLSN();
+       if (max_replication_slots > 0)
+               ReplicationSlotsComputeRequiredLSN();
 }


-- 
Regards,
Dilip Kumar
Google