inhibit_m_s_w_k_s_during_upgrade_2.txt

text/plain

Filename: inhibit_m_s_w_k_s_during_upgrade_2.txt
Type: text/plain
Part: 0
Message: Re: A recent message added to pg_upgade
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 99823df3c7..f7dc966600 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -52,6 +52,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/builtins.h"
+#include "utils/guc_hooks.h"
 
 /*
  * Replication slot on-disk data structure.
@@ -111,6 +112,26 @@ static void RestoreSlotFromDisk(const char *name);
 static void CreateSlotOnDisk(ReplicationSlot *slot);
 static void SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel);
 
+/*
+ * GUC check_hook for max_slot_wal_keep_size
+ *
+ * All WAL files on the publisher node must be retained during an upgrade to
+ * maintain connections from downstreams.  While pg_upgrade explicitly sets
+ * this variable to -1, there are ways for users to modify it. Ensure it
+ * remains unchanged for safety.  See InvalidatePossiblyObsoleteSlot() and
+ * start_postmaster() in pg_upgrade for more details.
+ */
+bool
+check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
+{
+	if (IsBinaryUpgrade && *newval != -1)
+	{
+		GUC_check_errdetail("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade.");
+		return false;
+	}
+	return true;
+}
+
 /*
  * Report shared-memory space needed by ReplicationSlotsShmemInit.
  */
@@ -1424,18 +1445,12 @@ InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
 		SpinLockRelease(&s->mutex);
 
 		/*
-		 * The logical replication slots shouldn't be invalidated as
-		 * max_slot_wal_keep_size GUC is set to -1 during the upgrade.
-		 *
-		 * The following is just a sanity check.
+		 * check_max_slot_wal_keep_size() ensures max_slot_wal_keep_size is set
+		 * to -1, so, slot invalidation for logical slots shouldn't happen
+		 * during an upgrade. At present, only logical slots really require
+		 * this.
 		 */
-		if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
-		{
-			ereport(ERROR,
-					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("replication slots must not be invalidated during the upgrade"),
-					errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"));
-		}
+		Assert (!*invalidated || !SlotIsLogical(s) || !IsBinaryUpgrade);
 
 		if (active_pid != 0)
 		{
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 7605eff9b9..818ffdb2ae 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2845,7 +2845,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&max_slot_wal_keep_size_mb,
 		-1, -1, MAX_KILOBYTES,
-		NULL, NULL, NULL
+		check_max_slot_wal_keep_size, NULL, NULL
 	},
 
 	{
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 2a191830a8..3d74483f44 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -84,6 +84,8 @@ extern bool check_maintenance_io_concurrency(int *newval, void **extra,
 extern void assign_maintenance_io_concurrency(int newval, void *extra);
 extern bool check_max_connections(int *newval, void **extra, GucSource source);
 extern bool check_max_wal_senders(int *newval, void **extra, GucSource source);
+extern bool check_max_slot_wal_keep_size(int *newval, void **extra,
+										 GucSource source);
 extern void assign_max_wal_size(int newval, void *extra);
 extern bool check_max_worker_processes(int *newval, void **extra,
 									   GucSource source);