Hi,
While reviewing another logical replication thread, I noticed an issue with altering a subscription's two_phase option.
In AlterSubscription(), executing ALTER SUBSCRIPTION ... SET (two_phase = true) unconditionally set subtwophasestate to LOGICALREP_TWOPHASE_STATE_PENDING, even when two_phase was already in LOGICALREP_TWOPHASE_STATE_ENABLED state.
Because two_phase remains enabled on the publisher's replication slot and prepared transactions may already exist on the subscriber, overwriting the state to PENDING broke checks that rely on subtwophasestate being ENABLED:
1. A subsequent ALTER SUBSCRIPTION ... SET (two_phase = false) skipped the LookupGXactBySubid() check and disabled two_phase while prepared transactions still existed on the subscriber. When those transactions were later committed on the publisher, the publisher re-sent all of their changes as regular transactions instead of COMMIT PREPARED, leading to self-deadlocks, duplicate data, or orphaned prepared transactions.
2. ALTER SUBSCRIPTION ... REFRESH PUBLICATION (and SET/ADD PUBLICATION) with copy_data = true was allowed even though the publisher's slot still had two_phase enabled, allowing prepared transactions on newly added tables to be streamed during initial table synchronization and applied as empty prepares.
I think we can fix this by not resetting subtwophasestate to PENDING when two_phase is already in the ENABLED state, as shown in the attached patch.
This issue has existed since PG 18, when altering the two_phase option was introduced, so it should be back-patched down to that branch.
-- Best Regards, Zhijie Hou
From 7afcf8a34aa479d2d81cc350352832b60ccaf844 Mon Sep 17 00:00:00 2001 From: Zhijie Hou <[email protected]> Date: Thu, 29 Oct 2026 13:24:32 +0800 Subject: [PATCH v1] Avoid resetting subscription two_phase state to pending when already enabled. In AlterSubscription(), executing ALTER SUBSCRIPTION ... SET (two_phase = true) unconditionally set subtwophasestate to LOGICALREP_TWOPHASE_STATE_PENDING ('p'), even when two_phase was already in LOGICALREP_TWOPHASE_STATE_ENABLED ('e') state. Because two_phase remains enabled on the publisher's replication slot and prepared transactions may already exist on the subscriber, overwriting the state to 'p' broke checks that rely on subtwophasestate being 'e': 1. A subsequent ALTER SUBSCRIPTION ... SET (two_phase = false) skipped the LookupGXactBySubid() check and disabled two_phase while prepared transactions still existed on the subscriber. When those transactions were later committed on the publisher, the publisher re-sent all of their changes as regular transactions instead of COMMIT PREPARED, leading to self-deadlocks, duplicate data, or orphaned prepared transactions. 2. ALTER SUBSCRIPTION ... REFRESH PUBLICATION (and SET/ADD PUBLICATION) with copy_data = true was allowed even though the publisher's slot still had two_phase enabled, allowing prepared transactions on newly added tables to be streamed during initial table synchronization and applied as empty prepares. Fix this by not resetting subtwophasestate to PENDING when two_phase is already in ENABLED state. Author: Zhijie Hou <[email protected]> Backpatch-through: 18 --- src/backend/commands/subscriptioncmds.c | 19 +++++++++++++------ src/test/subscription/t/021_twophase.pl | 10 ++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 36bb20a1f6e..560b601a407 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1963,12 +1963,19 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, errmsg("cannot disable \"two_phase\" when prepared transactions exist"), errhint("Resolve these transactions and try again."))); - /* Change system catalog accordingly */ - values[Anum_pg_subscription_subtwophasestate - 1] = - CharGetDatum(opts.twophase ? - LOGICALREP_TWOPHASE_STATE_PENDING : - LOGICALREP_TWOPHASE_STATE_DISABLED); - replaces[Anum_pg_subscription_subtwophasestate - 1] = true; + /* + * Change system catalog accordingly, but do not reset the + * state to PENDING if two_phase is already enabled. + */ + if (!opts.twophase || + sub->twophasestate != LOGICALREP_TWOPHASE_STATE_ENABLED) + { + values[Anum_pg_subscription_subtwophasestate - 1] = + CharGetDatum(opts.twophase ? + LOGICALREP_TWOPHASE_STATE_PENDING : + LOGICALREP_TWOPHASE_STATE_DISABLED); + replaces[Anum_pg_subscription_subtwophasestate - 1] = true; + } } if (IsSet(opts.specified_opts, SUBOPT_FAILOVER)) diff --git a/src/test/subscription/t/021_twophase.pl b/src/test/subscription/t/021_twophase.pl index f755e748e9c..eb85e8e43d3 100644 --- a/src/test/subscription/t/021_twophase.pl +++ b/src/test/subscription/t/021_twophase.pl @@ -447,6 +447,16 @@ $node_subscriber->safe_psql('postgres', $node_subscriber->poll_query_until('postgres', "SELECT count(*) = 0 FROM pg_stat_activity WHERE backend_type = 'logical replication apply worker'" ); + +# Verify that setting two_phase to true when it is already enabled does not +# overwrite subtwophasestate to 'p' (pending). +$node_subscriber->safe_psql('postgres', + "ALTER SUBSCRIPTION tap_sub_copy SET (two_phase = true);"); +$result = $node_subscriber->safe_psql('postgres', + "SELECT subtwophasestate FROM pg_subscription WHERE subname = 'tap_sub_copy';" +); +is($result, qq(e), 'two-phase subscription option remains enabled'); + $node_subscriber->safe_psql( 'postgres', " ALTER SUBSCRIPTION tap_sub_copy SET (two_phase = false); -- 2.54.0 (Apple Git-157)
