OK,
Now that the fix is committed, shall we introduce some of the asserts? I
believe there's an agreement the restart_lsn shouldn't move backwards,
so I propose the attached patch.
I still think not resetting the fields when releasing the slot, and
allowing the values to move backwards is rather suspicious. But I don't
have any reproducer demonstrating an issue (beyond just hitting an
assert). Perhaps it's correct, but in that case it'd be good to add a
comment explaining why that's the case. Sadly, it has to be written by
someone else - I've been unable to form a justification why it's OK :-(
regards
--
Tomas Vondra
From 25678fbbef96965dfb54387dacbe979c920b84e8 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@vondra.me>
Date: Tue, 19 Nov 2024 16:21:57 +0100
Subject: [PATCH vasserts 1/3] asserts: restart_lsn
---
src/backend/replication/logical/logical.c | 3 +++
src/backend/replication/slot.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index e941bb491d8..7fb9f7bd0fa 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -391,6 +391,7 @@ CreateInitDecodingContext(const char *plugin,
else
{
SpinLockAcquire(&slot->mutex);
+ Assert(slot->data.restart_lsn <= restart_lsn);
slot->data.restart_lsn = restart_lsn;
SpinLockRelease(&slot->mutex);
}
@@ -1877,6 +1878,8 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
{
Assert(MyReplicationSlot->candidate_restart_lsn != InvalidXLogRecPtr);
+ Assert(MyReplicationSlot->data.restart_lsn <= MyReplicationSlot->candidate_restart_lsn);
+
MyReplicationSlot->data.restart_lsn = MyReplicationSlot->candidate_restart_lsn;
MyReplicationSlot->candidate_restart_lsn = InvalidXLogRecPtr;
MyReplicationSlot->candidate_restart_valid = InvalidXLogRecPtr;
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 6828100cf1a..0f38ccc8353 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1468,6 +1468,7 @@ ReplicationSlotReserveWal(void)
restart_lsn = GetXLogInsertRecPtr();
SpinLockAcquire(&slot->mutex);
+ Assert(slot->data.restart_lsn <= restart_lsn);
slot->data.restart_lsn = restart_lsn;
SpinLockRelease(&slot->mutex);
--
2.47.0
