On Thu, Jul 9, 2026 at 9:23 PM Srinath Reddy Sadipiralla
<[email protected]> wrote:
>
>
>
> ---------- Forwarded message ---------
> From: Srinath Reddy Sadipiralla <[email protected]>
> Date: Fri, Jul 10, 2026 at 9:48 AM
> Subject: Fix "unexpected logical decoding status change" error; from 
> concurrent logical decoding activation
> To: PostgreSQL Hackers <[email protected]>
> Cc: Masahiko Sawada <[email protected]>
>
>
> Hi,
>
> While stress-testing REPACK CONCURRENTLY, I was in that
> area looking at BUG #19519 (REPACK failing with "missingchunk
> number N for toast value" [0]; note that one reproduces with plain
> REPACK too, since it's the shared copy_table_data() path, so
> it's independent of what follows), I hit a different, reproducible
> failure under high concurrency:
>     ERROR:  unexpected logical decoding status change 1
>
> I believe this is a race bug in the on-demand logical decoding
> activation machinery (logicalctl.c), exposed by
> REPACK CONCURRENTLY but not actually specific to it, as
> this can be reproduced using pg_create_logical_replication_slot and 
> CREATE_REPLICATION_SLOT.
>
> At wal_level = 'replica', logical decoding is toggled on the first time a
> logical slot is created.  EnableLogicalDecoding() sets the shared
> logical_decoding_enabled flag and writes an
> XLOG_LOGICAL_DECODING_STATUS_CHANGE record so standbys
> learn about the change.  On the decoding side, xlog_decode() treats
> that record as unreachable:
> elog(ERROR, "unexpected logical decoding status change %d", ...);
>
> The reasoning (per the comment there) is that no running decoder can ever
> have such a record within the LSN range it scans: there is exactly one
> enable record per disabled->enabled transition, and it precedes the
> decoding start point reserved by any slot.
>
> That case does not hold under concurrency.  EnableLogicalDecoding()
> does:
>
>     LWLockAcquire(LogicalDecodingControlLock, EXCLUSIVE);
>     if (logical_decoding_enabled)     /* already on? -> done */
>         { ... return; }
>     LWLockRelease(...);
>
>     WaitForProcSignalBarrier(...);    /* lock released here */
>
>     LWLockAcquire(LogicalDecodingControlLock, EXCLUSIVE);
>     logical_decoding_enabled = true;
>     write_logical_decoding_status_update_record(true);   /* the record */
>     LWLockRelease(...);
>
> The "already enabled?" check and the record write happen under two
> separate lock acquisitions, with the barrier wait in between.  The lock
> must be dropped across the barrier; backends absorbing the barrier
> take LogicalDecodingControlLock in shared mode (via
> IsXLogLogicalInfoEnabled()), so holding it across WaitForProcSignalBarrier()
> would deadlock.
>
> So if two backends create the first logical slot(s) at the same time,
> both can pass the initial check while the flag is still false, and both
> end up writing a status-change record.  The second (redundant) record
> lands after the decoding start point the other slot already reserved,
> so that slot scans it and trips the elog() above.
>
> REPACK CONCURRENTLY makes this easy to hit because each operation
> creates a temporary logical slot (max_repack_replication_slots), so firing
> many REPACKs in parallel from the disabled state gives you many backends
> racing to perform the very first activation.  But nothing here is 
> REPACK-specific.
>
> To reproduce this i have added a test using logical-decoding-activation
> injection point in 051_effective_wal_level.pl
>
> I also hit it with a stress script [1] which was again related to the
> BUG #19519, instead of vacuum full i replaced it with REPACK (concurrently).
>
> To fix this I re-checked the flag after re-acquiring the lock and skipped
> the state change/WAL write if another backend had already completed
> the transition.
>
>     LWLockAcquire(LogicalDecodingControlLock, EXCLUSIVE);
>
>     if (LogicalDecodingCtl->logical_decoding_enabled)
>     {
>         LogicalDecodingCtl->pending_disable = false;
>         LWLockRelease(LogicalDecodingControlLock);
>         return;
>     }
>
>     START_CRIT_SECTION();
>
> With this, only the backend that actually performs the disabled->enabled
> transition writes a record, and every slot's start point stays at or after
> it, restoring the case xlog_decode() depends on.
>
> Patch attached.  It fixes logicalctl.c and adds a regression test to
> 051_effective_wal_level.pl reusing the existing injection-point
> infrastructure.  With the fix the test passes; without it it fails with
> the exact error above.

Thank you for the report and the patch!

I agree with your analysis and the patch basically looks good to me.
Here are some review comments:

+ # 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)
+]);

I don't think we need to call pg_log_standby_snapshot() until the slot
is created since the slot creation writes the running-xacts record
during the slot creation.

---
+ # 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");

We can use safe_psql() to check if the query successfully completes.

I've made some cosmetic changes to the comment and the new test
including the above comments. Please review it.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From 94979530b57adc2a68f48109fffcb30339da5ff0 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Tue, 14 Jul 2026 12:22:24 -0700
Subject: [PATCH v2] Fix race condition when enabling logical decoding
 concurrently.

With wal_level = 'replica', logical decoding is enabled on demand
when the first logical replication slot is created:

When enabling logical decoding, EnableLogicalDecoding() flips the
shared logical_decoding_enabled flag and writes an
XLOG_LOGICAL_DECODING_STATUS_CHANGE record so that standbys follow the
status change. The initial "already enabled?" check and the WAL record
write happen under two separate acquisitions of
LogicalDecodingControlLock, since the lock must be released while
waiting for the ProcSignalBarrier: processes absorbing the barrier
acquire the same lock in shared mode.

Consequently, if two backends concurrently created the first logical
slots, both could pass the initial check and both write a
status-change record. The redundant record lands after the decoding
start point already reserved by the other backend's slot, so decoding
that slot processes the record and fails with "unexpected logical
decoding status change", as xlog_decode() assumes that no such record
can appear within the WAL range any slot decodes.

Fix by re-checking the status after re-acquiring the lock, so that
only the backend that actually performs the disabled->enabled
transition writes the WAL record.

Reported-by: Srinath Reddy Sadipiralla <[email protected]>
Author: Srinath Reddy Sadipiralla <[email protected]>
Discussion: CAFC+b6oYzmAgp7F0ivrhfZT46-CjvCTrU9pWuMNcem-52YjOTw@mail.gmail.com">https://postgr.es/m/CAFC+b6oYzmAgp7F0ivrhfZT46-CjvCTrU9pWuMNcem-52YjOTw@mail.gmail.com
Backpatch-through: 19
---
 src/backend/replication/logical/logicalctl.c  | 11 ++++
 .../recovery/t/051_effective_wal_level.pl     | 61 ++++++++++++++++++-
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c
index c11d1316450..7074900514a 100644
--- a/src/backend/replication/logical/logicalctl.c
+++ b/src/backend/replication/logical/logicalctl.c
@@ -384,6 +384,17 @@ EnableLogicalDecoding(void)
 
 	LWLockAcquire(LogicalDecodingControlLock, LW_EXCLUSIVE);
 
+	/*
+	 * Re-check whether logical decoding got enabled while we waited for the
+	 * barrier above.
+	 */
+	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..2cf2ea6546d 100644
--- a/src/test/recovery/t/051_effective_wal_level.pl
+++ b/src/test/recovery/t/051_effective_wal_level.pl
@@ -326,11 +326,12 @@ $standby3->safe_psql('postgres',
 
 $standby3->stop;
 
-# Test the race condition at end of the recovery between the startup and logical
-# decoding status change. This test requires injection points enabled.
 if (   $ENV{enable_injection_points} eq 'yes'
 	&& $primary->check_extension('injection_points'))
 {
+	# Test the race condition at end of the recovery between the startup and logical
+	# decoding status change. This test requires injection points enabled.
+
 	# Initialize standby4 and start it.
 	my $standby4 = PostgreSQL::Test::Cluster->new('standby4');
 	$standby4->init_from_backup($primary, 'my_backup', has_streaming => 1);
@@ -381,9 +382,63 @@ if (   $ENV{enable_injection_points} eq 'yes'
 	test_wal_level($primary, "replica|replica",
 		"effective_wal_level got decreased to 'replica' on primary");
 
+	# Test that concurrent activations don't write redundant status-change records.
+
+	# Start a psql session and stop it in the middle of the activation process.
+	my $psql_create_slot = $primary->background_psql('postgres');
+	$psql_create_slot->query_until(
+		qr/create_slot_1/,
+		q(\echo create_slot_1
+select injection_points_set_local();
+select injection_points_attach('logical-decoding-activation', 'wait');
+select pg_create_logical_replication_slot('slot_1', 'test_decoding');
+));
+	$primary->wait_for_event('client backend', 'logical-decoding-activation');
+	note("injection_point 'logical-decoding-activation' is reached");
+
+	# A second backend concurrently enables logical decoding and finishes creating
+	# its slot, writing the status-change record. The slot reserves its decoding
+	# start point after its own status-change record.
+	$primary->safe_psql('postgres',
+		qq[select pg_create_logical_replication_slot('slot_2', 'test_decoding')]
+	);
+	test_wal_level($primary, "replica|logical",
+		"logical decoding enabled by the first of two concurrent activations"
+	);
+
+	# Resume the first backend to complete the slot creation. It must not write
+	# a second redundant status-change record as logical decoding is already
+	# enabled.
+	$primary->safe_psql('postgres',
+		qq[select injection_points_wakeup('logical-decoding-activation')]);
+
+	# Let the released backend finish creating its slot.
+	$psql_create_slot->quit;
+
+	# Decode from slot_2, whose start point precedes where a redundant
+	# status-change record would have been written; this fails in xlog_decode()
+	# if one exists.
+	is( $primary->safe_psql(
+			'postgres',
+			qq[SELECT count(*) FROM pg_logical_slot_get_changes('slot_2', NULL, NULL, 'skip-empty-xacts', '1')]
+		),
+		0,
+		'decoding a concurrently-created slot succeeds');
+
+	# Restore the disabled state for the tests that follow.
+	$primary->safe_psql(
+		'postgres',
+		qq[
+select pg_drop_replication_slot('slot_1');
+select pg_drop_replication_slot('slot_2');
+]);
+	wait_for_logical_decoding_disabled($primary);
+
+	# Test a race when logical decoding activation is concurrently interrupted.
+
 	# Start a psql session to test the case where the activation process is
 	# interrupted.
-	my $psql_create_slot = $primary->background_psql('postgres');
+	$psql_create_slot = $primary->background_psql('postgres');
 
 	# Start the logical decoding activation process upon creating the logical
 	# slot, but it will wait due to the injection point.
-- 
2.54.0

Reply via email to