From 225b279313d91d025d3fd3675e485c268e499537 Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Mon, 19 Oct 2026 19:09:06 -0400 Subject: [PATCH v5] Skip the end-of-recovery barrier wait when nothing changed If logical decoding stays enabled through recovery, only processes started before StartupLogicalDecodingStatus() can hold a stale cache. Emit a barrier there and wait for it at the end of recovery, so a backend blocked on a stuck client cannot hold up promotion. --- src/backend/replication/logical/logicalctl.c | 39 +++++- src/test/recovery/meson.build | 1 + .../recovery/t/061_promote_stuck_client.pl | 123 ++++++++++++++++++ 3 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 src/test/recovery/t/061_promote_stuck_client.pl diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c index 7b4b54e5935..af806c59003 100644 --- a/src/backend/replication/logical/logicalctl.c +++ b/src/backend/replication/logical/logicalctl.c @@ -142,6 +142,12 @@ bool XLogLogicalInfo = false; */ static bool XLogLogicalInfoUpdatePending = false; +/* + * Generation of the barrier emitted by StartupLogicalDecodingStatus(), or 0 + * if it emitted none. Only meaningful in the startup process. + */ +static uint64 startup_status_generation = 0; + static void update_xlog_logical_info(void); static void abort_logical_decoding_activation(int code, Datum arg); static void write_logical_decoding_status_update_record(bool status); @@ -183,6 +189,15 @@ StartupLogicalDecodingStatus(bool last_status) */ LogicalDecodingCtl->xlog_logical_info = last_status; LogicalDecodingCtl->logical_decoding_enabled = last_status; + + /* + * Processes started before this point, such as the checkpointer, cached + * the old status. Tell them to refresh, and remember the barrier so that + * the end of recovery can wait for it. + */ + if (IsUnderPostmaster && last_status) + startup_status_generation = + EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO); } /* @@ -672,6 +687,7 @@ void UpdateLogicalDecodingStatusEndOfRecovery(void) { bool new_status = false; + bool status_changed; Assert(RecoveryInProgress()); @@ -715,7 +731,8 @@ UpdateLogicalDecodingStatusEndOfRecovery(void) * by the time of the recheck, the backend instead enables logical * decoding by itself, which is fine after promotion.) */ - if (new_status != LogicalDecodingCtl->logical_decoding_enabled) + status_changed = (new_status != LogicalDecodingCtl->logical_decoding_enabled); + if (status_changed) { /* * Update both the logical decoding status and logical WAL logging @@ -761,9 +778,23 @@ UpdateLogicalDecodingStatusEndOfRecovery(void) { uint64 generation; - generation = EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO); - if (new_status) - WaitForProcSignalBarrier(generation); + /* + * If the status was enabled all along, a stale cache can only be in a + * process started before StartupLogicalDecodingStatus(), because + * later processes read the enabled status at startup and an enable + * replayed in between waited for everyone. So waiting for the + * startup barrier is enough. This keeps a backend that cannot absorb + * barriers, e.g. one blocked writing to a client, from holding up + * promotion. + */ + if (new_status && !status_changed) + WaitForProcSignalBarrier(startup_status_generation); + else + { + generation = EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO); + if (new_status) + WaitForProcSignalBarrier(generation); + } } INJECTION_POINT("startup-logical-decoding-status-change-end-of-recovery", NULL); diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index 87dbdb6326e..58789bfe430 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -68,6 +68,7 @@ tests += { 't/057_snapshot_commit_race.pl', 't/059_remote_apply_status_interval.pl', 't/060_multixact_crashed_updater.pl', + 't/061_promote_stuck_client.pl', ], }, } diff --git a/src/test/recovery/t/061_promote_stuck_client.pl b/src/test/recovery/t/061_promote_stuck_client.pl new file mode 100644 index 00000000000..592ee097f23 --- /dev/null +++ b/src/test/recovery/t/061_promote_stuck_client.pl @@ -0,0 +1,123 @@ + +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Check that promotion of a standby that keeps logical decoding enabled is +# not held up by a backend blocked writing to a client that stopped reading. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Promotion normally takes well under a second. Without the fix it never +# finishes while the client is stuck. +my $promote_timeout = int($PostgreSQL::Test::Utils::timeout_default / 6); + +sub promote_with_stuck_client +{ + my ($standby, $name) = @_; + + is( $standby->safe_psql('postgres', 'SHOW effective_wal_level'), + 'logical', "$name: logical decoding is enabled on the standby"); + + # psql writes the COPY data to a pipe that we never read, so the backend + # fills its socket buffer and blocks. + my ($in, $out, $err) = ('', '', ''); + my $writer = IPC::Run::start( + [ + 'psql', '--no-psqlrc', + '--dbname' => $standby->connstr('postgres'), + '--command' => + "COPY (SELECT repeat('x', 1000) FROM generate_series(1, 1000000)) TO STDOUT" + ], + '<' => \$in, + '>' => \$out, + '2>' => \$err); + + # Without this the test passes when the output fits in the buffers. + $standby->poll_query_until('postgres', + "SELECT count(*) = 1 FROM pg_stat_activity WHERE wait_event = 'ClientWrite'" + ) or die "timed out waiting for a backend to block in ClientWrite"; + + is( $standby->safe_psql( + 'postgres', "SELECT pg_promote(true, $promote_timeout)"), + 't', + "$name: promotion completes with a backend blocked on a write"); + + $writer->kill_kill; + $standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'); +} + +# Logical decoding is enabled by replay, and the slot is created on the +# standby. +{ + my $primary = PostgreSQL::Test::Cluster->new('primary_a'); + $primary->init(allows_streaming => 1); + $primary->start; + $primary->backup('backup'); + + my $standby = PostgreSQL::Test::Cluster->new('standby_a'); + $standby->init_from_backup($primary, 'backup', has_streaming => 1); + $standby->start; + + $primary->safe_psql('postgres', + "SELECT pg_create_logical_replication_slot('primary_slot', 'test_decoding')" + ); + $primary->wait_for_replay_catchup($standby); + $standby->create_logical_slot_on_standby($primary, 'standby_slot', + 'postgres'); + + promote_with_stuck_client($standby, 'enabled by replay'); + + $standby->stop; + $primary->stop; +} + +# Logical decoding is enabled when the standby starts, and the slot comes +# from slot synchronization. +{ + my $primary = PostgreSQL::Test::Cluster->new('primary_b'); + $primary->init(allows_streaming => 1); + $primary->start; + $primary->safe_psql('postgres', + "SELECT pg_create_physical_replication_slot('sb1');" + . "SELECT pg_create_logical_replication_slot('keep', 'test_decoding');" + ); + $primary->backup('backup'); + + my $standby = PostgreSQL::Test::Cluster->new('standby_b'); + $standby->init_from_backup($primary, 'backup', has_streaming => 1); + my $connstr = $primary->connstr . ' dbname=postgres'; + $standby->append_conf( + 'postgresql.conf', qq( +primary_slot_name = 'sb1' +hot_standby_feedback = on +primary_conninfo = '$connstr' +)); + $standby->start; + $primary->wait_for_replay_catchup($standby); + + # Slot synchronization needs the catalog xmin held by the feedback. + $primary->poll_query_until('postgres', + "SELECT xmin IS NOT NULL OR catalog_xmin IS NOT NULL FROM pg_replication_slots WHERE slot_name = 'sb1'" + ); + $primary->safe_psql('postgres', + "SELECT pg_create_logical_replication_slot('fslot', 'test_decoding', false, false, true)" + ); + $primary->wait_for_replay_catchup($standby); + $standby->safe_psql('postgres', 'SELECT pg_sync_replication_slots()'); + is( $standby->safe_psql( + 'postgres', + "SELECT synced FROM pg_replication_slots WHERE slot_name = 'fslot'"), + 't', + 'failover slot is synced to the standby'); + + promote_with_stuck_client($standby, 'enabled at startup'); + + $standby->stop; + $primary->stop; +} + +done_testing(); -- 2.37.1 (Apple Git-137.1)