From cc628f66187c56e0bfd1befee298755f7dfbf65e Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Mon, 24 Feb 2025 14:03:10 -0800 Subject: [PATCH] Fix assertion when decoding XLOG_PARAMETER_CHANGE on promoted primary. When a standby replays an XLOG_PARAMETER_CHANGE record that lowers wal_level below logical, we invalidate all logical slots in hot standby mode. However, if this record was replayed while not in hot standby mode, logical slots could remain valid even after promotion, potentially causing an assertion failure during WAL record decoding. To fix this issue, this commit adds a check for hot_standby status when restoring a logical replication slot on standbys. This check ensures that logical slots are invalidated when they become incompatible due to insufficient wal_level during recovery. Backpatch to v16 where logical decoding on standby was introduced. Reviewed-by: Amit Kapila Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/CAD21AoABoFwGY_Rh2aeE6tEq3HkJxf0c6UeOXn4VV9v6BAQPSw%40mail.gmail.com Backpatch-through: 16 --- src/backend/replication/slot.c | 29 +++++++++++--- .../t/035_standby_logical_decoding.pl | 38 +++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 1f9d0ed9b3e..a223d80f715 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -2071,12 +2071,29 @@ RestoreSlotFromDisk(const char *name) * NB: Changing the requirements here also requires adapting * CheckSlotRequirements() and CheckLogicalDecodingRequirements(). */ - if (cp.slotdata.database != InvalidOid && wal_level < WAL_LEVEL_LOGICAL) - ereport(FATAL, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("logical replication slot \"%s\" exists, but wal_level < logical", - NameStr(cp.slotdata.name)), - errhint("Change wal_level to be logical or higher."))); + if (cp.slotdata.database != InvalidOid) + { + if (wal_level < WAL_LEVEL_LOGICAL) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("logical replication slot \"%s\" exists, but wal_level < logical", + NameStr(cp.slotdata.name)), + errhint("Change wal_level to be logical or higher."))); + + /* + * In standby mode, the hot standby must be enabled. This check is + * necessary to ensure logical slots are invalidated when they become + * incompatible due to insufficient wal_level. Otherwise, if the + * primary reduces wal_level < logical while hot standby is disabled, + * logical slots would remain valid even after promotion. + */ + if (StandbyMode && !EnableHotStandby) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("logical replication slot \"%s\" exists on the standby, but hot_standby = off", + NameStr(cp.slotdata.name)), + errhint("Change hot_standby to be on."))); + } else if (wal_level < WAL_LEVEL_REPLICA) ereport(FATAL, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl index e90191fe2aa..8120dfc2132 100644 --- a/src/test/recovery/t/035_standby_logical_decoding.pl +++ b/src/test/recovery/t/035_standby_logical_decoding.pl @@ -345,6 +345,44 @@ $psql_subscriber{run} = IPC::Run::start( \$psql_subscriber{subscriber_stderr}, IPC::Run::timeout($default_timeout)); +################################################## +# Test that the standby requires hot_standby to be +# enabled for pre-existing logical slots. +################################################## + +# create the logical slots +$node_standby->create_logical_slot_on_standby($node_primary, 'restart_test'); +$node_standby->stop; +$node_standby->append_conf('postgresql.conf', qq[hot_standby = off]); + +# Use run_log instead of $node_standby->start because this test expects +# that the server ends with an error during startup. +run_log( + [ + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', + ]); + +# wait for postgres to terminate +foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default) +{ + last if !-f $node_standby->data_dir . '/postmaster.pid'; + usleep(100_000); +} + +# Confirm that the server startup fails with an expected error +my $logfile = slurp_file($node_standby->logfile()); +ok( $logfile =~ + qr/FATAL: .* logical replication slot ".*" exists on the standby, but hot_standby = off/, + "the standby ends with an error during startup because hot_standby was disabled" +); +$node_standby->adjust_conf('postgresql.conf', 'hot_standby', 'on'); +$node_standby->start; +$node_standby->safe_psql('postgres', + qq[SELECT pg_drop_replication_slot('restart_test')]); + ################################################## # Test that logical decoding on the standby # behaves correctly.