public inbox for [email protected]
help / color / mirror / Atom feedFrom: Vitaly Davydov <[email protected]>
To: [email protected]
Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly
Date: Thu, 31 Oct 2024 13:32:36 +0300
Message-ID: <1d2043-67235d00-5-2e3223c0@131762497> (raw)
In-Reply-To: <1d12d2-67235980-35-19a406a0@63439497>
References: <1d12d2-67235980-35-19a406a0@63439497>
Sorry, attached the missed patch.
On Thursday, October 31, 2024 13:18 MSK, "Vitaly Davydov" <[email protected]> wrote:
Dear Hackers,
I'd like to discuss a problem with replication slots's restart LSN. Physical slots are saved to disk at the beginning of checkpoint. At the end of checkpoint, old WAL segments are recycled or removed from disk, if they are not kept by slot's restart_lsn values.
If an existing physical slot is advanced in the middle of checkpoint execution, WAL segments, which are related to saved on disk restart LSN may be removed. It is because the calculation of the replication slot miminal LSN is occured at the end of checkpoint, prior to old WAL segments removal. If to hard stop (pg_stl -m immediate) the postgres instance right after checkpoint and to restart it, the slot's restart_lsn may point to the removed WAL segment. I believe, such behaviour is not good.
The doc [0] describes that restart_lsn may be set to the some past value after reload. There is a discussion [1] on pghackers where such behaviour is discussed. The main reason of not flushing physical slots on advancing is a performance reason. I'm ok with such behaviour, except of that the corresponding WAL segments should not be removed.
I propose to keep WAL segments by saved on disk (flushed) restart_lsn of slots. Add a new field restart_lsn_flushed into ReplicationSlot structure. Copy restart_lsn to restart_lsn_flushed in SaveSlotToPath. It doesn't change the format of storing the slot contents on disk. I attached a patch. It is not yet complete, but demonstate a way to solve the problem.
I reproduced the problem by the following way:
* Add some delay in CheckPointBuffers (pg_usleep) to emulate long checkpoint execution. * Execute checkpoint and pg_replication_slot_advance right after starting of the checkpoint from another connection. * Hard restart the server right after checkpoint completion. * After restart slot's restart_lsn may point to removed WAL segment.
The proposed patch fixes it.
[0] https://www.postgresql.org/docs/current/logicaldecoding-explanation.html
[1] https://www.postgresql.org/message-id/flat/059cc53a-8b14-653a-a24d-5f867503b0ee%40postgrespro.ru
Attachments:
[text/x-patch] 0001-Keep-WAL-segments-by-slot-s-flushed-restart-LSN.patch (2.1K, ../1d2043-67235d00-5-2e3223c0@131762497/3-0001-Keep-WAL-segments-by-slot-s-flushed-restart-LSN.patch)
download | inline diff:
From acae6b55fc766d2fe1bfe85eb8af85110f55dcc8 Mon Sep 17 00:00:00 2001
From: Vitaly Davydov <[email protected]>
Date: Thu, 31 Oct 2024 12:29:12 +0300
Subject: [PATCH] Keep WAL segments by slot's flushed restart LSN
---
src/backend/replication/slot.c | 9 +++++++--
src/include/replication/slot.h | 4 ++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 6828100cf1..ee7ab3678e 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1148,7 +1148,9 @@ ReplicationSlotsComputeRequiredLSN(void)
continue;
SpinLockAcquire(&s->mutex);
- restart_lsn = s->data.restart_lsn;
+ restart_lsn = s->restart_lsn_flushed != InvalidXLogRecPtr ?
+ s->restart_lsn_flushed :
+ s->data.restart_lsn;
invalidated = s->data.invalidated != RS_INVAL_NONE;
SpinLockRelease(&s->mutex);
@@ -1207,7 +1209,9 @@ ReplicationSlotsComputeLogicalRestartLSN(void)
/* read once, it's ok if it increases while we're checking */
SpinLockAcquire(&s->mutex);
- restart_lsn = s->data.restart_lsn;
+ restart_lsn = s->restart_lsn_flushed != InvalidXLogRecPtr ?
+ s->restart_lsn_flushed :
+ s->data.restart_lsn;
invalidated = s->data.invalidated != RS_INVAL_NONE;
SpinLockRelease(&s->mutex);
@@ -2097,6 +2101,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
SpinLockAcquire(&slot->mutex);
memcpy(&cp.slotdata, &slot->data, sizeof(ReplicationSlotPersistentData));
+ slot->restart_lsn_flushed = slot->data.restart_lsn;
SpinLockRelease(&slot->mutex);
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 45582cf9d8..ca4c3aab3b 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -207,6 +207,10 @@ typedef struct ReplicationSlot
/* The time since the slot has become inactive */
TimestampTz inactive_since;
+
+ /* Latest restart LSN that was flushed to disk */
+ XLogRecPtr restart_lsn_flushed;
+
} ReplicationSlot;
#define SlotIsPhysical(slot) ((slot)->data.database == InvalidOid)
--
2.34.1
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly
In-Reply-To: <1d2043-67235d00-5-2e3223c0@131762497>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox