v33-0002-FIXUP-address-review-comments.patch

application/octet-stream

Filename: v33-0002-FIXUP-address-review-comments.patch
Type: application/octet-stream
Part: 1
Message: Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v33-0002
Subject: FIXUP: address review comments.
File+
doc/src/sgml/config.sgml 3 3
src/backend/access/transam/xlog.c 3 3
src/backend/replication/logical/decode.c 12 27
src/backend/replication/logical/logicalctl.c 4 1
src/backend/replication/slot.c 3 2
From 431100a49eb448c07c07c2a2f1b71aa6511cc477 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
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</link>. The system automatically increases the
         effective WAL level to <literal>logical</literal> when creating the first
         logical replication slot, and decreases it back to <literal>replica</literal>
-        when dropping the last logical replication slot. The current effective WAL
-        level can be monitored through <xref linkend="guc-effective-wal-level"/>
-        parameter.
+        when dropping or invalidating the last logical replication slot. The current
+        effective WAL level can be monitored through
+        <xref linkend="guc-effective-wal-level"/> parameter.
        </para>
        <para>
         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