From 4ae6d960b209d88120558d65379abb739d53f22d Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 6 Apr 2023 19:51:06 -0700 Subject: [PATCH va65 2/9] Prevent use of invalidated logical slot in CreateDecodingContext() Previously we had checks for this in multiple places. Support for logical decoding on standbys will add further checks, making it worth while to change this. TODO: Ensure the error message differences across walsender / SQL interface are the right thing. As far as I can tell, the "This slot has never previously reserved WAL" portion in pg_logical_slot_get_changes_guts() was bogus. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/replication/logical/logical.c | 16 ++++++++++++++++ src/backend/replication/logical/logicalfuncs.c | 13 ------------- src/backend/replication/walsender.c | 7 ------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index c3ec97a0a62..85fc49f655d 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -518,6 +518,22 @@ CreateDecodingContext(XLogRecPtr start_lsn, errmsg("replication slot \"%s\" was not created in this database", NameStr(slot->data.name)))); + /* + * Check if slot has been invalidated due to max_slot_wal_keep_size. Avoid + * "cannot get changes" wording in this errmsg because that'd be + * confusingly ambiguous about no changes being available when called from + * pg_logical_slot_get_changes_guts(). + */ + if (MyReplicationSlot->data.invalidated == RS_INVAL_WAL) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("can no longer get changes from replication slot \"%s\"", + NameStr(MyReplicationSlot->data.name)), + errdetail("This slot has been invalidated because it exceeded the maximum reserved size."))); + + Assert(MyReplicationSlot->data.invalidated == RS_INVAL_NONE); + Assert(MyReplicationSlot->data.restart_lsn != InvalidXLogRecPtr); + if (start_lsn == InvalidXLogRecPtr) { /* continue from last position */ diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c index fa1b641a2b0..55a24c02c94 100644 --- a/src/backend/replication/logical/logicalfuncs.c +++ b/src/backend/replication/logical/logicalfuncs.c @@ -214,19 +214,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin LogicalOutputPrepareWrite, LogicalOutputWrite, NULL); - /* - * After the sanity checks in CreateDecodingContext, make sure the - * restart_lsn is valid. Avoid "cannot get changes" wording in this - * errmsg because that'd be confusingly ambiguous about no changes - * being available. - */ - if (XLogRecPtrIsInvalid(MyReplicationSlot->data.restart_lsn)) - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("can no longer get changes from replication slot \"%s\"", - NameStr(*name)), - errdetail("This slot has never previously reserved WAL, or it has been invalidated."))); - MemoryContextSwitchTo(oldcontext); /* diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 75e8363e248..e40a9b1ba7b 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -1253,13 +1253,6 @@ StartLogicalReplication(StartReplicationCmd *cmd) ReplicationSlotAcquire(cmd->slotname, true); - if (XLogRecPtrIsInvalid(MyReplicationSlot->data.restart_lsn)) - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("cannot read from logical replication slot \"%s\"", - cmd->slotname), - errdetail("This slot has been invalidated because it exceeded the maximum reserved size."))); - /* * Force a disconnect, so that the decoding code doesn't need to care * about an eventual switch from running in recovery, to running in a -- 2.38.0