From 23b38de310e83dab59998ef22e14b2b0cb233e0e Mon Sep 17 00:00:00 2001 From: Dilip Kumar 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