From 431100a49eb448c07c07c2a2f1b71aa6511cc477 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Thu, 4 Dec 2025 10:51:25 -0800 Subject: [PATCH v33 2/2] FIXUP: address review comments. --- doc/src/sgml/config.sgml | 6 +-- src/backend/access/transam/xlog.c | 6 +-- src/backend/replication/logical/decode.c | 39 ++++++-------------- src/backend/replication/logical/logicalctl.c | 5 ++- src/backend/replication/slot.c | 5 ++- 5 files changed, 25 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 58f7aefcbae..40907c6385c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3053,9 +3053,9 @@ include_dir 'conf.d' logical replication slots. The system automatically increases the effective WAL level to logical when creating the first logical replication slot, and decreases it back to replica - when dropping the last logical replication slot. The current effective WAL - level can be monitored through - parameter. + when dropping or invalidating the last logical replication slot. The current + effective WAL level can be monitored through + parameter. In releases prior to 9.6, this parameter also allowed the diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7da634b8d4d..920fc4822c6 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8696,6 +8696,9 @@ xlog_redo(XLogReaderState *record) /* The record must always change the status actually */ Assert(IsLogicalDecodingEnabled() != logical_decoding); + /* Update the status on shared memory */ + UpdateLogicalDecodingStatus(logical_decoding, true); + if (InRecovery && InHotStandby) { if (!logical_decoding) @@ -8721,9 +8724,6 @@ xlog_redo(XLogReaderState *record) kill(PostmasterPid, SIGUSR1); } } - - /* Update the status on shared memory */ - UpdateLogicalDecodingStatus(logical_decoding, true); } } diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index b96aacd3676..b551722fd68 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -149,15 +149,6 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) * can restart from there. */ break; - case XLOG_PARAMETER_CHANGE: - - /* - * A XLOG_PARAMETER_CHANGE record might change wal_level to - * 'replica' or 'minimal', but we don't check the logical decoding - * availability here because it's updated by a - * XLOG_LOGICAL_DECODING_STATUS_CHANGE record. - */ - break; case XLOG_LOGICAL_DECODING_STATUS_CHANGE: { bool logical_decoding; @@ -165,25 +156,18 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) memcpy(&logical_decoding, XLogRecGetData(buf->record), sizeof(bool)); /* - * Existing logical slots on the standby get invalidated when - * this WAL record is replayed; and further, slot creation - * fails when logical decoding is disabled; but all these - * operations are not synchronized, so a logical slot may - * creep in while the logical decoding status is being - * disabled. Hence this extra check. + * Error out as we should not decode this WAL record. + * + * Logical decoding is disabled and then existing logical + * slots on the standby get invalidated when this WAL record + * is replayed. No logical slot can creep in while the logical + * decoding status is being disabled. This cannot occur even + * on the primary either, as the primary would not allow to + * restart after changing wal_level < replica if there is + * pre-existing logical slot. */ - if (!logical_decoding) - { - /* - * This can occur only on a standby, as a primary would - * not allow to restart after changing wal_level < replica - * if there is pre-existing logical slot. - */ - Assert(RecoveryInProgress()); - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("logical decoding on standby requires \"effective_wal_level\" >= \"logical\" on the primary"))); - } + elog(ERROR, "unexpected logical decoding status change %d", + logical_decoding); break; } @@ -191,6 +175,7 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) case XLOG_NEXTOID: case XLOG_SWITCH: case XLOG_BACKUP_END: + case XLOG_PARAMETER_CHANGE: case XLOG_RESTORE_POINT: case XLOG_FPW_CHANGE: case XLOG_FPI_FOR_HINT: diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c index cef8410ef76..5bc2f79f2e9 100644 --- a/src/backend/replication/logical/logicalctl.c +++ b/src/backend/replication/logical/logicalctl.c @@ -364,7 +364,10 @@ EnsureLogicalDecodingEnabled(void) LWLockAcquire(LogicalDecodingControlLock, LW_EXCLUSIVE); - /* Return if someone already started to enable logical decoding */ + /* + * Return if someone already started to enable logical decoding, or if it + * is already enabled. + */ if (LogicalDecodingCtl->xlog_logical_info) { LogicalDecodingCtl->pending_disable = false; diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index f8f9b03b297..67c15918a7a 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -2790,8 +2790,9 @@ RestoreSlotFromDisk(const char *name) * In standby mode, the hot standby must be enabled. This check is * necessary to ensure logical slots are invalidated when they become * incompatible due to insufficient wal_level. Otherwise, if the - * primary reduces wal_level < logical while hot standby is disabled, - * logical slots would remain valid even after promotion. + * primary reduces effective_wal_level < logical while hot standby is + * disabled, primary disable logical decoding while hot standby is + * disabled, logical slots would remain valid even after promotion. */ if (StandbyMode && !EnableHotStandby) ereport(FATAL, -- 2.47.3