Prohibit max_slot_wal_keep_size to value other than -1 during upgrade.

We don't want existing slots in the old cluster to get invalidated during
the upgrade. During an upgrade, we set this variable to -1 via the command
line in an attempt to prevent such invalidations, but users have ways to
override it. This patch ensures the value is not overridden by the user.

Author: Kyotaro Horiguchi
Reviewed-by: Peter Smith, Alvaro Herrera
Discussion: http://postgr.es/m/20231027.115759.2206827438943188717[email protected]

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b541be8eec213c391a2c4b5af084c861430e954a..1159dff1a692c619104d8c75db17633ec55bb6f3 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2063,6 +2063,25 @@ 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/replication/slot.c b/src/backend/replication/slot.c
index 99823df3c7d8f4cffeb3fdbef209206b2495d059..781aa43cc4fcaa2f6c5072f3a3019c9176132fae 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1424,18 +1424,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.
+                * The logical replication slots shouldn't be invalidated as GUC
+                * max_slot_wal_keep_size is set to -1 during the binary upgrade. See
+                * check_old_cluster_for_valid_slots() where we ensure that no
+                * invalidated before the upgrade.
                 */
-               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 beed72abbd6a1356f0e3723d969344093616714f..b764ef69980aed7c5495c9d25ef076e18d97bf54 100644 (file)
--- 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 2a191830a89ffc3f74fdff3509f6f342f309213f..3d74483f447859d6656c3943db340893bc2d55e4 100644 (file)
--- 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);