Fix the handling of two GUCs during upgrade.

Previously, the check_hook functions for max_slot_wal_keep_size and
idle_replication_slot_timeout would incorrectly raise an ERROR for values
set in postgresql.conf during upgrade, even though those values were not
actively used in the upgrade process.

To prevent logical slot invalidation during upgrade, we used to set
special values for these GUCs. Now, instead of relying on those values, we
directly prevent WAL removal and logical slot invalidation caused by
max_slot_wal_keep_size and idle_replication_slot_timeout.

Note: PostgreSQL 17 does not include the idle_replication_slot_timeout
GUC, so related changes were not backported.

BUG #18979
Reported-by: jorsol <jorsol@gmail.com>
Author: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed by: vignesh C <vignesh21@gmail.com>
Reviewed by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Backpatch-through: 17, where it was introduced
Discussion: https://postgr.es/m/219561.1751826409@sss.pgh.pa.us
Discussion: https://postgr.es/m/18979-a1b7fdbb7cd181c6@postgresql.org

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a00786d40c8068805a9593da539673052eb251c7..ffea4993177c6318b509885b0ed66f26e6d6373e 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2215,25 +2215,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.
@@ -7992,17 +7973,19 @@ KeepLogSeg(XLogRecPtr recptr, XLogRecPtr slotsMinReqLSN, XLogSegNo *logSegNo)
        XLByteToSeg(recptr, currSegNo, wal_segment_size);
        segno = currSegNo;
 
-       /*
-        * Calculate how many segments are kept by slots first, adjusting for
-        * max_slot_wal_keep_size.
-        */
+       /* Calculate how many segments are kept by slots. */
        keep = slotsMinReqLSN;
        if (keep != InvalidXLogRecPtr && keep < recptr)
        {
                XLByteToSeg(keep, segno, wal_segment_size);
 
-               /* Cap by max_slot_wal_keep_size ... */
-               if (max_slot_wal_keep_size_mb >= 0)
+               /*
+                * Account for max_slot_wal_keep_size to avoid keeping more than
+                * configured.  However, don't do that during a binary upgrade: if
+                * slots were to be invalidated because of this, it would not be
+                * possible to preserve logical ones during the upgrade.
+                */
+               if (max_slot_wal_keep_size_mb >= 0 && !IsBinaryUpgrade)
                {
                        uint64          slot_keep_segs;
 

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index a1d4768623f55aecd39a18932ed8f48b608a7700..a234e2ca9c1af7273af42b3d17412580e83bbc24 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1671,14 +1671,6 @@ InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
 
                SpinLockRelease(&s->mutex);
 
-               /*
-                * 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.
-                */
-               Assert(!(*invalidated && SlotIsLogical(s) && IsBinaryUpgrade));
-
                if (active_pid != 0)
                {
                        /*
@@ -1805,6 +1797,10 @@ restart:
                if (!s->in_use)
                        continue;
 
+               /* Prevent invalidation of logical slots during binary upgrade. */
+               if (SlotIsLogical(s) && IsBinaryUpgrade)
+                       continue;
+
                if (InvalidatePossiblyObsoleteSlot(cause, s, oldestLSN, dboid,
                                                                                   snapshotConflictHorizon,
                                                                                   &invalidated))

diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c42fccf3fe74d40d590e89003e8352504b2bc948..a997dcb7dbcb7a7d6aaf2b2b96913214e5394048 100644 (file)
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2953,7 +2953,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/server.c b/src/bin/pg_upgrade/server.c
index 91bcb4dbc7e694e8ec7d2a905c58dfc8c3c36167..b223d5afddfffd91e33e531b2990b48991bed883 100644 (file)
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -241,17 +241,6 @@ start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error)
        if (cluster == &new_cluster)
                appendPQExpBufferStr(&pgoptions, " -c synchronous_commit=off -c fsync=off -c full_page_writes=off");
 
-       /*
-        * Use max_slot_wal_keep_size as -1 to prevent the WAL removal by the
-        * checkpointer process.  If WALs required by logical replication slots
-        * are removed, the slots are unusable.  This setting prevents the
-        * invalidation of slots during the upgrade. We set this option when
-        * cluster is PG17 or later because logical replication slots can only be
-        * migrated since then. Besides, max_slot_wal_keep_size is added in PG13.
-        */
-       if (GET_MAJOR_VERSION(cluster->major_version) >= 1700)
-               appendPQExpBufferStr(&pgoptions, " -c max_slot_wal_keep_size=-1");
-
        /*
         * Use -b to disable autovacuum and logical replication launcher
         * (effective in PG17 or later for the latter).

diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 8fd91af3887f762305681b04cabcf5dbf07825e8..1babff78bf35c840b6f8fee0d317bdf73106c5d1 100644 (file)
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -86,8 +86,6 @@ 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);