From 235a1c590cf85636bce155c3092b1e0f95a48fc0 Mon Sep 17 00:00:00 2001 From: Srinath Reddy Sadipiralla Date: Fri, 10 Jul 2026 09:12:19 +0530 Subject: [PATCH 1/1] Avoid writing a redundant logical-decoding status-change record When wal_level = 'replica', logical decoding is enabled on demand the first time a logical slot is created.EnableLogicalDecoding() flips the shared logical_decoding_enabled flag and writes an XLOG_LOGICAL_DECODING_STATUS_CHANGE record to propagate the change to standbys. xlog_decode() treats that record as unreachable and errors out, on the assumption that exactly one such record is written per disabled->enabled transition and that it always precedes the decoding start point of every slot. That assumption can be violated.EnableLogicalDecoding() checks logical_decoding_enabled, then releases LogicalDecodingControlLock to wait on a ProcSignalBarrier, then re-acquires the lock to write the record.The lock must be released across the barrier wait, because the backends absorbing the barrier take it in shared mode.If two backends create the first logical slot(s) concurrently, both can observe the flag still unset before either writes the record, so both perform the transition and both emit a status-change record.The second, redundant record is written after the decoding start point that the other slot has already reserved, so that slot decodes it and fails with: ERROR: unexpected logical decoding status change 1 Fix by re-checking logical_decoding_enabled after re-acquiring the lock, and skipping the state change and the WAL write if another backend has already completed the transition.This mirrors the existing fast-path check at the top of the function. Add a regression test to 051_effective_wal_level.pl that drives the race deterministically using the logical-decoding-activation injection point. Reported-by: Srinath Reddy Sadipiralla Author: Srinath Reddy Sadipiralla --- src/backend/replication/logical/logicalctl.c | 22 ++++++ .../recovery/t/051_effective_wal_level.pl | 71 +++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c index c11d1316450..40e86086954 100644 --- a/src/backend/replication/logical/logicalctl.c +++ b/src/backend/replication/logical/logicalctl.c @@ -384,6 +384,28 @@ EnableLogicalDecoding(void) LWLockAcquire(LogicalDecodingControlLock, LW_EXCLUSIVE); + /* + * Re-check whether logical decoding got enabled while we waited for the + * barrier above. We had to release the lock across the barrier wait (the + * backends absorbing the barrier take this lock in shared mode, so holding + * it here would deadlock), which means a concurrent activator may have + * completed the disabled->enabled transition in the meantime. In that + * case the WAL record has already been written and we must not write + * another one: a second, redundant XLOG_LOGICAL_DECODING_STATUS_CHANGE + * record could fall within the WAL range that a concurrently-created slot + * has already reserved as its decoding start point, and xlog_decode() + * treats such a record as unreachable and errors out. + * + * This mirrors the fast-path check at the top of this function; we simply + * have to repeat it now that we hold the lock again. + */ + if (LogicalDecodingCtl->logical_decoding_enabled) + { + LogicalDecodingCtl->pending_disable = false; + LWLockRelease(LogicalDecodingControlLock); + return; + } + START_CRIT_SECTION(); /* diff --git a/src/test/recovery/t/051_effective_wal_level.pl b/src/test/recovery/t/051_effective_wal_level.pl index d4bc7f0aa40..6a31a25e4a8 100644 --- a/src/test/recovery/t/051_effective_wal_level.pl +++ b/src/test/recovery/t/051_effective_wal_level.pl @@ -381,6 +381,77 @@ if ( $ENV{enable_injection_points} eq 'yes' test_wal_level($primary, "replica|replica", "effective_wal_level got decreased to 'replica' on primary"); + # A redundant activation must not leave a stray status-change record that a + # concurrently-created slot later decodes. Two backends racing to enable + # logical decoding can both pass the "already enabled?" check in + # EnableLogicalDecoding() while the flag is still off (the lock is released + # across the barrier wait) and each write an + # XLOG_LOGICAL_DECODING_STATUS_CHANGE record. The second, redundant record + # lands after the decoding start point already reserved by the first slot, + # which then decodes it and errors out in xlog_decode() with "unexpected + # logical decoding status change". Logical decoding is disabled here, and + # this runs before the cancellation tests below so that no stale waiter is + # left registered on the injection point (a canceled waiter is not cleaned + # up, which would misdirect injection_points_wakeup()). + + # Park one activator right after the barrier wait but before it flips the + # flag and writes the record. + my $psql_stray = $primary->background_psql('postgres'); + $psql_stray->query_until( + qr/create_slot_stray/, + q(\echo create_slot_stray +select injection_points_set_local(); +select injection_points_attach('logical-decoding-activation', 'wait'); +select pg_create_logical_replication_slot('slot_stray', 'test_decoding'); +)); + $primary->wait_for_event('client backend', 'logical-decoding-activation'); + note("injection_point 'logical-decoding-activation' is reached"); + + # A second backend enables logical decoding and finishes creating its slot, + # reserving its decoding start point after its own status-change record (so + # it never decodes its own record). + $primary->safe_psql('postgres', + qq[select pg_create_logical_replication_slot('slot_first', 'test_decoding')] + ); + test_wal_level($primary, "replica|logical", + "logical decoding enabled by the first of two concurrent activations"); + + # Release the parked activator so it runs the rest of the activation code. + # Logical decoding is already enabled by now, so it must NOT write a second, + # redundant status-change record. In the buggy case it writes one here, + # just after slot_first's reserved decoding start point. + $primary->safe_psql('postgres', + qq[select injection_points_wakeup('logical-decoding-activation')]); + + # Let the released backend finish creating its slot: feed running-xacts + # records until it reaches a consistent point (poll_query_until re-runs the + # query, so pg_log_standby_snapshot() is called until the slot is created). + $primary->poll_query_until( + 'postgres', qq[ +select (pg_log_standby_snapshot() is not null) + and exists (select 1 from pg_replication_slots + where slot_name = 'slot_stray' and confirmed_flush_lsn is not null) +]); + $psql_stray->quit; + + # Decoding the first slot must not stumble over a stray status-change record. + my ($decode_rc, $decode_out, $decode_err) = $primary->psql( + 'postgres', + qq[select count(*) from pg_logical_slot_get_changes('slot_first', null, null)], + on_error_die => 0); + is($decode_rc, 0, "decoding a concurrently-created slot succeeds"); + unlike( + $decode_err, + qr/unexpected logical decoding status change/, + "no redundant status-change record was decoded"); + + # Restore the disabled state for the tests that follow. + $primary->safe_psql('postgres', + qq[select pg_drop_replication_slot('slot_stray')]); + $primary->safe_psql('postgres', + qq[select pg_drop_replication_slot('slot_first')]); + wait_for_logical_decoding_disabled($primary); + # Start a psql session to test the case where the activation process is # interrupted. my $psql_create_slot = $primary->background_psql('postgres'); -- 2.43.0