On Wed, Sep 30, 2026 at 11:28 PM Hayato Kuroda (Fujitsu) <[email protected]> wrote: > > Dear Sawada-san, Serigei, > > Good catch, I have also been seeing and considering the test, but Sawada-san > is faster. > > > The fix looks good to me. We need to check and update XLogLogicalInfo > > in every place where we reset the top-level transaction id. > > > > I've updated the patch with the regression tests. Please review it. > > Confirmed the test fails on HEAD and pass after the patch. > There might be idea to put the function after the "nParallelCurrentXids = 0;" > even in the CommitTransaction() and PrepareTransaction(), which is same as > CleanupTransaction(). But any places are OK for me. > > LGTM.
Thank you for reviewing the patch. I'd rather move AtEOXact_LogicalCtl() to before resetting CurrentResourceOwner. I think it should work fine too and caon deal with your concern. I've attached the updated patch. I'm going to push it on Monday, barring objections. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
From 04fba40d382cffdc43abb51694df8fcf9bc00b6c Mon Sep 17 00:00:00 2001 From: Masahiko Sawada <[email protected]> Date: Wed, 30 Sep 2026 20:06:20 -0700 Subject: [PATCH v2] Fix deferred XLogLogicalInfo update after a failed transaction block. Previously, a backend with an assigned XID deferred the XLogLogicalInfo update requested by a procsignal barrier, and we applied it in AbortTransaction() on abort. However, a failed transaction block keeps its XID until ROLLBACK, so a barrier absorbed while idle in that state was deferred, and CleanupTransaction() did not apply it. The next transaction then ran entirely with the stale value. If logical decoding had just been enabled, that transaction's changes were written without logical information and silently skipped by decoding. Fix by applying the pending update in CleanupTransaction() instead of AbortTransaction(). Patch by Sergei Patiakin, with tests added by me. Oversight in commit 67c20979ce7. Reported-by: Sergei Patiakin <[email protected]> Author: Sergei Patiakin <[email protected]> Reviewed-by: Shlok Kyal <[email protected]> Reviewed-by: Hayato Kuroda <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://postgr.es/m/cane55rapenafaqxlwrvm-nc0y5gkrbfzfvahvyp89ags2sy...@mail.gmail.com Backpatch-through: 19 --- src/backend/access/transam/xact.c | 8 ++++- src/backend/replication/logical/logicalctl.c | 10 ++++++- .../recovery/t/051_effective_wal_level.pl | 30 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 7b67db514ec..feac096d735 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3054,7 +3054,6 @@ AbortTransaction(void) AtEOXact_PgStat(false, is_parallel_worker); AtEOXact_ApplyLauncher(false); AtEOXact_LogicalRepWorkers(false); - AtEOXact_LogicalCtl(); pgstat_report_xact_timestamp(0); } @@ -3085,6 +3084,13 @@ CleanupTransaction(void) AtCleanup_Portals(); /* now safe to release portal memory */ AtEOXact_Snapshot(false, true); /* and release the transaction's snapshots */ + /* + * Apply any pending XLogLogicalInfo update. This is done here rather than + * in AbortTransaction(), as a failed transaction block keeps its XID + * until we get here. See AtEOXact_LogicalCtl(). + */ + AtEOXact_LogicalCtl(); + CurrentResourceOwner = NULL; /* and resource owner */ if (TopTransactionResourceOwner) ResourceOwnerDelete(TopTransactionResourceOwner); diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c index 642d965bd1c..15beebca796 100644 --- a/src/backend/replication/logical/logicalctl.c +++ b/src/backend/replication/logical/logicalctl.c @@ -227,7 +227,15 @@ IsXLogLogicalInfoEnabled(void) } /* - * Reset the local cache at end of the transaction. + * Apply a pending XLogLogicalInfo update at end of the top-level transaction. + * + * This is called from CommitTransaction(), PrepareTransaction(), and + * CleanupTransaction(), which are the only places where the top-level XID is + * reset, so the next transaction always starts with the latest value. Note + * that AbortTransaction() is not a suitable place, because a failed + * transaction block keeps its XID until ROLLBACK, and a barrier absorbed in + * between is deferred. Subtransaction ends don't call this either, as the + * value must stay the same until the top-level transaction ends. */ void AtEOXact_LogicalCtl(void) diff --git a/src/test/recovery/t/051_effective_wal_level.pl b/src/test/recovery/t/051_effective_wal_level.pl index b11690863d9..2f2719d4286 100644 --- a/src/test/recovery/t/051_effective_wal_level.pl +++ b/src/test/recovery/t/051_effective_wal_level.pl @@ -78,6 +78,36 @@ wait_for_logical_decoding_disabled($primary); test_wal_level($primary, "replica|replica", "logical decoding disabled after repack"); +# Test that a backend applies an XLogLogicalInfo update that it received +# while its transaction block was in the failed state, once the block ends. +# The failed transaction keeps its XID until ROLLBACK, so the update is +# deferred; it must not be carried over into the next transaction. +my $psql_aborted = $primary->background_psql('postgres', on_error_stop => 0); +$psql_aborted->query_safe(q[begin; select pg_current_xact_id();]); +my ($aborted_out, $aborted_ret) = $psql_aborted->query(q[select 1/0;]); +is($aborted_ret, 1, "transaction block failed"); +$psql_aborted->{stderr} = ''; + +# Enable logical decoding while the backend is idle in the failed transaction +# block. This waits for all backends to absorb the barrier. +$primary->safe_psql('postgres', + qq[select pg_create_logical_replication_slot('test_aborted_slot', 'test_decoding')] +); + +# Check the value in the transaction right after ROLLBACK. Note that both +# commands need to be sent together, as query_safe() appends an empty query +# that would run in its own transaction and apply the deferred update. +is( $psql_aborted->query_safe( + q[rollback; select current_setting('effective_wal_level');]), + 'logical', + "effective_wal_level is updated after rolling back a failed transaction block" +); +$psql_aborted->quit; + +$primary->safe_psql('postgres', + qq[select pg_drop_replication_slot('test_aborted_slot')]); +wait_for_logical_decoding_disabled($primary); + # Create a new logical slot and check that effective_wal_level must be increased # to 'logical'. $primary->safe_psql('postgres', -- 2.55.0
