v1-0001-pg_upgrade-Accurately-validate-max_slot_wal_keep_.patch

From 23b38de310e83dab59998ef22e14b2b0cb233e0e Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 7 Jul 2025 04:58:25 +0000
Subject: [PATCH v1] pg_upgrade: Accurately validate max_slot_wal_keep_size

This commit improves the accuracy of the 'max_slot_wal_keep_size'
check during pg_upgrade. Previously, a generic GUC hook would
check regardless of the parameter's final configured value.

Now, pg_upgrade directly queries the server for 'max_slot_wal_keep_size'
after it starts. An error is raised only if the resulting value is not -1,
preventing unnecessary error.
---
 src/backend/access/transam/xlog.c   | 19 -----------------
 src/backend/utils/misc/guc_tables.c |  2 +-
 src/bin/pg_upgrade/check.c          | 33 +++++++++++++++++++++++++++++
 src/include/utils/guc_hooks.h       |  2 --
 4 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 47ffc0a2307..62cca52eba9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2346,25 +2346,6 @@ check_wal_segment_size(int *newval, void **extra, GucSource source)
 	return true;
 }
 
-/*
- * GUC check_hook for max_slot_wal_keep_size
- *
- * We don't allow the value of max_slot_wal_keep_size other than -1 during the
- * binary upgrade. See 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("\"%s\" must be set to -1 during binary upgrade mode.",
-							"max_slot_wal_keep_size");
-		return false;
-	}
-
-	return true;
-}
-
 /*
  * At a checkpoint, how many WAL segments to recycle as preallocated future
  * XLOG segments? Returns the highest segment that should be preallocated.
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 511dc32d519..e59400abdf2 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3081,7 +3081,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&max_slot_wal_keep_size_mb,
 		-1, -1, MAX_KILOBYTES,
-		check_max_slot_wal_keep_size, NULL, NULL
+		NULL, NULL, NULL
 	},
 
 	{
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index fb063a2de42..89f25b486d5 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,6 +29,7 @@ static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static void check_for_unicode_update(ClusterInfo *cluster);
 static void check_new_cluster_logical_replication_slots(void);
 static void check_new_cluster_subscription_configuration(void);
+static void check_max_slot_wal_keep_size(ClusterInfo *cluster);
 static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 
@@ -599,6 +600,9 @@ check_and_dump_old_cluster(void)
 	 */
 	check_for_connection_status(&old_cluster);
 
+	/* Validate max_slot_wal_keep_size is set to -1 or not. */
+	check_max_slot_wal_keep_size(&old_cluster);
+
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
@@ -701,6 +705,8 @@ check_and_dump_old_cluster(void)
 void
 check_new_cluster(void)
 {
+	check_max_slot_wal_keep_size(&new_cluster);
+
 	get_db_rel_and_slot_infos(&new_cluster);
 
 	check_new_cluster_is_empty();
@@ -2059,6 +2065,33 @@ check_new_cluster_subscription_configuration(void)
 	check_ok();
 }
 
+/*
+ * check_max_slot_wal_keep_size()
+ *
+ * We don't allow the value of max_slot_wal_keep_size other than -1 during the
+ * binary upgrade. See start_postmaster() in pg_upgrade for more details.
+ */
+static void
+check_max_slot_wal_keep_size(ClusterInfo *cluster)
+{
+	PGconn	   *conn;
+	PGresult   *res;
+	int			wal_keep_size;
+
+	if (GET_MAJOR_VERSION(cluster->major_version) < 1700)
+		return;
+
+	conn = connectToServer(cluster, "template1");
+	res = executeQueryOrDie(conn, "SELECT setting FROM pg_settings "
+							"WHERE name = 'max_slot_wal_keep_size';");
+	wal_keep_size = atoi(PQgetvalue(res, 0, 0));
+	if (wal_keep_size != -1)
+		pg_fatal("\"max_slot_wal_keep_size\" (%d) must be set to -1", wal_keep_size);
+
+	PQclear(res);
+	PQfinish(conn);
+}
+
 /*
  * check_old_cluster_for_valid_slots()
  *
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 799fa7ace68..feda19bc243 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -84,8 +84,6 @@ extern const char *show_log_timezone(void);
 extern void assign_maintenance_io_concurrency(int newval, void *extra);
 extern void assign_io_max_combine_limit(int newval, void *extra);
 extern void assign_io_combine_limit(int newval, void *extra);
-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_stack_depth(int *newval, void **extra, GucSource source);
 extern void assign_max_stack_depth(int newval, void *extra);
-- 
2.50.0.727.gbf7dc18ff4-goog