diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index b541be8eec..46833f6ecd 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2063,6 +2063,29 @@ check_wal_segment_size(int *newval, void **extra, GucSource source) return true; } +/* + * GUC check_hook for max_slot_wal_keep_size + * + * If WALs needed by logical replication slots are deleted, these slots become + * inoperable. During a binary upgrade, pg_upgrade sets this variable to -1 via + * the command line in an attempt to prevent such deletions, but users have + * ways to override it. To ensure the successful completion of the upgrade, + * it's essential to keep this variable unaltered. 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("\"%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 99823df3c7..5c3d2b1082 100644 --- 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. + * 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/bin/pg_upgrade/t/003_logical_slots.pl b/src/bin/pg_upgrade/t/003_logical_slots.pl index 5b01cf8c40..1a55a75827 100644 --- a/src/bin/pg_upgrade/t/003_logical_slots.pl +++ b/src/bin/pg_upgrade/t/003_logical_slots.pl @@ -22,7 +22,36 @@ $oldpub->init(allows_streaming => 'logical'); my $newpub = PostgreSQL::Test::Cluster->new('newpub'); $newpub->init(allows_streaming => 'logical'); -# Setup a common pg_upgrade command to be used by all the test cases +# In a VPATH build, we'll be started in the source directory, but we want +# to run pg_upgrade in the build directory so that any files generated finish +# in it, like delete_old_cluster.{sh,bat}. +chdir ${PostgreSQL::Test::Utils::tmp_check}; + +# ------------------------------ +# TEST: Confirm max_slot_wal_keep_size must not be overwritten + +# pg_upgrade will fail because the GUC max_slot_wal_keep_size is overwritten +# to a positive value +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '-d', $oldpub->data_dir, + '-D', $newpub->data_dir, + '-b', $oldpub->config_data('--bindir'), + '-B', $newpub->config_data('--bindir'), + '-s', $newpub->host, + '-p', $oldpub->port, + '-P', $newpub->port, + $mode, '-o " -c max_slot_wal_keep_size=1MB"' + ], + 1, + [qr/could not connect to source postmaster started with the command/], + [qr//], + 'run of pg_upgrade where max_slot_wal_keep_size is overwritten.'); +ok(-d $newpub->data_dir . "/pg_upgrade_output.d", + "pg_upgrade_output.d/ not removed after pg_upgrade failure"); + +# Setup a common pg_upgrade command to be used by upcoming test cases my @pg_upgrade_cmd = ( 'pg_upgrade', '--no-sync', '-d', $oldpub->data_dir, @@ -34,11 +63,6 @@ my @pg_upgrade_cmd = ( '-P', $newpub->port, $mode); -# In a VPATH build, we'll be started in the source directory, but we want -# to run pg_upgrade in the build directory so that any files generated finish -# in it, like delete_old_cluster.{sh,bat}. -chdir ${PostgreSQL::Test::Utils::tmp_check}; - # ------------------------------ # TEST: Confirm pg_upgrade fails when the new cluster has wrong GUC values @@ -67,8 +91,6 @@ command_checks_all( [qr//], 'run of pg_upgrade where the new cluster has insufficient max_replication_slots' ); -ok( -d $newpub->data_dir . "/pg_upgrade_output.d", - "pg_upgrade_output.d/ not removed after pg_upgrade failure"); # Set 'max_replication_slots' to match the number of slots (2) present on the # old cluster. Both slots will be used for subsequent tests. 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);