On Tue, Apr 29, 2025 at 2:51 PM shveta malik <shveta.ma...@gmail.com> wrote:
>
> On Mon, Apr 28, 2025 at 4:33 PM Nisha Moond <nisha.moond...@gmail.com> wrote:
> >
> > Please find the v9 patch, addressing the above and all other comments as
> > well.
> >
>
> Thanks for the patch.
>
> 1)
>
> + The default is false. This option cannot be set together with
> + <literal>two_phase</literal> when creating the slot. However, once
> + the slot is created with <literal>two_phase=true</literal>, the
> + <literal>failover</literal> can be set to true after the
> + <literal>restart_lsn</literal> has advanced beyond its
> + <literal>two_phase_at</literal> value
>
> As the user is not aware of two_phase_at, we shall change all the docs
> to have a more user friendly explanation.
>
Done.
> 2)
> # Confirm that the invalidated slot has been dropped.
> $standby1->wait_for_log(
> - qr/dropped replication slot "lsub1_slot" of database with OID
> [0-9]+/, $log_offset);
> + qr/dropped replication slot "lsub1_slot" of database with OID [0-9]+/,
> + $log_offset);
>
> I think this change is not needed.
>
The perltidy is auto-correcting it now, though it didn't for earlier
patches. I've kept the original for now.
~~~
Thanks for review, please find the updated patch v10.
--
Thanks,
Nisha
From 7aa643c49411f9b855ee01aacb28c6f646d6dc1f Mon Sep 17 00:00:00 2001
From: Nisha Moond <nisha.moond412@gmail.com>
Date: Mon, 7 Apr 2025 09:36:29 +0530
Subject: [PATCH v10] PG17 Approach 3 Fix slot synchronization for two_phase
enables slots.
The issue is that the transactions prepared before two-phase decoding is
enabled can fail to replicate to the subscriber after being committed on a
promoted standby following a failover. This is because the two_phase_at
field of a slot, which tracks the LSN from which two-phase decoding
starts, is not synchronized to standby servers. Without two_phase_at, the
logical decoding might incorrectly identify prepared transaction as
already replicated to the subscriber after promotion of standby server,
causing them to be skipped.
To prevent the risk of losing prepared transactions, we disallow enabling
both failover and twophase during slot creation, but permits altering
failover to true once ensured that slot's restart_lsn > two_phase_at.
The fix enforces the following conditions:
1) Always disallow creating slots with two_phase=true and failover=true.
2) Always disallow creating subscriptions with (two_phase=true, failover=true).
3) Prevent altering the slot's failover to true if two_phase=true and restart_lsn
is less than two_phase_at. Otherwise, allow changing failover to true.
4) Disallow altering slot's failover to true when two_phase state is 'pending'.
User can try altering failover again when two_phase state is moved beyond 'pending'.
---
contrib/test_decoding/expected/slot.out | 2 +
contrib/test_decoding/sql/slot.sql | 1 +
doc/src/sgml/func.sgml | 10 +-
doc/src/sgml/protocol.sgml | 14 +-
doc/src/sgml/ref/alter_subscription.sgml | 8 ++
doc/src/sgml/ref/create_subscription.sgml | 9 ++
src/backend/commands/subscriptioncmds.c | 43 +++++++
src/backend/replication/logical/logical.c | 12 ++
src/backend/replication/slot.c | 37 ++++++
src/bin/pg_upgrade/t/003_logical_slots.pl | 6 +-
.../t/040_standby_failover_slots_sync.pl | 121 ++++++++++++++++++
src/test/regress/expected/subscription.out | 4 +
src/test/regress/sql/subscription.sql | 4 +
13 files changed, 263 insertions(+), 8 deletions(-)
diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out
index 7de03c79f6f..87b28ad8d55 100644
--- a/contrib/test_decoding/expected/slot.out
+++ b/contrib/test_decoding/expected/slot.out
@@ -427,6 +427,8 @@ SELECT 'init' FROM pg_create_logical_replication_slot('failover_default_slot', '
SELECT 'init' FROM pg_create_logical_replication_slot('failover_true_temp_slot', 'test_decoding', true, false, true);
ERROR: cannot enable failover for a temporary replication slot
+SELECT 'init' FROM pg_create_logical_replication_slot('failover_twophase_true_slot', 'test_decoding', false, true, true);
+ERROR: cannot enable both "failover" and "two_phase" options during replication slot creation
SELECT 'init' FROM pg_create_physical_replication_slot('physical_slot');
?column?
----------
diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sql
index 580e3ae3bef..a89fe712ff6 100644
--- a/contrib/test_decoding/sql/slot.sql
+++ b/contrib/test_decoding/sql/slot.sql
@@ -182,6 +182,7 @@ SELECT 'init' FROM pg_create_logical_replication_slot('failover_true_slot', 'tes
SELECT 'init' FROM pg_create_logical_replication_slot('failover_false_slot', 'test_decoding', false, false, false);
SELECT 'init' FROM pg_create_logical_replication_slot('failover_default_slot', 'test_decoding', false, false);
SELECT 'init' FROM pg_create_logical_replication_slot('failover_true_temp_slot', 'test_decoding', true, false, true);
+SELECT 'init' FROM pg_create_logical_replication_slot('failover_twophase_true_slot', 'test_decoding', false, true, true);
SELECT 'init' FROM pg_create_physical_replication_slot('physical_slot');
SELECT slot_name, slot_type, failover FROM pg_replication_slots;
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f441ec43314..e0d70117202 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -29073,9 +29073,13 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
<parameter>failover</parameter>, when set to true,
specifies that this slot is enabled to be synced to the
standbys so that logical replication can be resumed after
- failover. A call to this function has the same effect as
- the replication protocol command
- <literal>CREATE_REPLICATION_SLOT ... LOGICAL</literal>.
+ failover. The parameters <parameter>twophase</parameter> and
+ <parameter>failover</parameter> cannot be enabled together when
+ creating a replication slot. However, a slot created with <parameter>twophase</parameter>
+ enabled can later have <parameter>failover</parameter> set to true via
+ <command>ALTER SUBSCRIPTION</command>, if a subscription is using this
+ slot. A call to this function has the same effect as the replication
+ protocol command <literal>CREATE_REPLICATION_SLOT ... LOGICAL</literal>.
</para></entry>
</row>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 0396d3a9e98..c87c60b74a6 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2060,7 +2060,12 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
and <literal>ROLLBACK PREPARED</literal> are decoded and transmitted.
The transaction will be decoded and transmitted at
<literal>PREPARE TRANSACTION</literal> time.
- The default is false.
+ The default is false. This option cannot be set together with
+ <literal>failover</literal> when creating a logical replication slot.
+ However, once the slot is created with <literal>two_phase = true</literal>,
+ <literal>failover</literal> can be set to true after the slot has
+ consumed all transactions up to the point where two-phase decoding
+ was enabled.
</para>
</listitem>
</varlistentry>
@@ -2101,7 +2106,12 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
<para>
If true, the slot is enabled to be synced to the standbys
so that logical replication can be resumed after failover.
- The default is false.
+ The default is false. This option cannot be set together with
+ <literal>two_phase</literal> when creating the slot. However, once
+ the slot is created with <literal>two_phase = true</literal>,
+ <literal>failover</literal> can be set to true after the slot has
+ consumed all transactions up to the point where two-phase decoding
+ was enabled.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 2ccbf5e4897..00e1c87e9c9 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -257,6 +257,14 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
<link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
option is enabled.
</para>
+
+ <para>
+ When <literal>two_phase</literal> is in the pending state, altering
+ <literal>failover = true</literal> is not permitted. Once
+ <literal>two_phase</literal> is enabled, <literal>failover = true</literal>
+ can be set only if the slot has consumed all transactions up to the point
+ where two-phase decoding was enabled.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index c9c8dd440dc..567b241f1e7 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -426,6 +426,15 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
replication can be resumed from the new primary after failover.
The default is <literal>false</literal>.
</para>
+
+ <para>
+ The options <literal>failover</literal> and <literal>two_phase</literal>
+ cannot be enabled together when creating a subscription. However, a
+ <literal>two_phase</literal> enabled subscription can later have
+ <literal>failover</literal> set to true via <command>ALTER SUBSCRIPTION</command>,
+ once the slot has consumed all transactions up to the point where
+ two-phase decoding was enabled.
+ </para>
</listitem>
</varlistentry>
</variablelist></para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..6e2830603ac 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -648,6 +648,21 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
errmsg("password_required=false is superuser-only"),
errhint("Subscriptions with the password_required option set to false may only be created or modified by the superuser.")));
+ /*
+ * Do not allow users to enable the failover and two_phase options
+ * together.
+ *
+ * See comments atop the similar check in ReplicationSlotCreate() for a
+ * detailed reason.
+ */
+ if (opts.twophase && opts.failover)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable both \"%s\" and \"%s\" options at CREATE SUBSCRIPTION",
+ "failover", "two_phase"),
+ errhint("The \"%s\" option can be enabled after \"%s\" state is ready using ALTER SUBSCRIPTION.",
+ "failover", "two_phase"));
+
/*
* If built with appropriate switch, whine when regression-testing
* conventions for subscription names are violated.
@@ -1245,6 +1260,34 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
errmsg("cannot set %s for enabled subscription",
"failover")));
+ /*
+ * Do not allow users to enable the failover when the
+ * two_phase state is still pending i.e., the replication
+ * slot’s two_phase option has not yet been finalized.
+ *
+ * In most cases, the restriction in
+ * ReplicationSlotAlter() is sufficient to prevent
+ * enabling failover for a slot with two_phase enabled.
+ * However, this additional check is necessary to handle a
+ * race condition, when a user runs CREATE SUBSCRIPTION
+ * with two_phase=true, but the slot's two_phase flag
+ * hasn't been set yet, a concurrent attempt to do ALTER
+ * SUBSCRIPTION(failover=true) may bypass the check in
+ * ReplicationSlotAlter().
+ *
+ * For a detailed explanation of why enforcing this
+ * restriction is important when combining two_phase and
+ * failover, refer to the comments atop similar check in
+ * ReplicationSlotCreate().
+ */
+ if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_PENDING &&
+ opts.failover)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot enable failover for a subscription with a pending two_phase state"),
+ errhint("The \"%s\" option can be enabled after \"%s\" state is ready.",
+ "failover", "two_phase"));
+
/*
* The changed failover option of the slot can't be rolled
* back.
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index e941bb491d8..2421e03d088 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -613,6 +613,18 @@ CreateDecodingContext(XLogRecPtr start_lsn,
/* Mark slot to allow two_phase decoding if not already marked */
if (ctx->twophase && !slot->data.two_phase)
{
+ /*
+ * Do not allow two-phase decoding for failover enabled slots.
+ *
+ * See comments atop the similar check in ReplicationSlotCreate() for
+ * a detailed reason.
+ */
+ if (slot->data.failover)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable two-phase decoding for failover enabled slot \"%s\"",
+ NameStr(slot->data.name))));
+
SpinLockAcquire(&slot->mutex);
slot->data.two_phase = true;
slot->data.two_phase_at = start_lsn;
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index a1d4768623f..6b763ae5dc6 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -343,6 +343,26 @@ ReplicationSlotCreate(const char *name, bool db_specific,
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot enable failover for a temporary replication slot"));
+
+ /*
+ * Do not allow users to enable both failover and two_phase for slots.
+ *
+ * This is because the two_phase_at field of a slot, which tracks the
+ * LSN, from which two-phase decoding starts, is not synchronized to
+ * standby servers. Without two_phase_at, the logical decoding might
+ * incorrectly identify prepared transaction as already replicated to
+ * the subscriber after promotion of standby server, causing them to
+ * be skipped.
+ *
+ * However, both failover and two_phase enabled slots can be created
+ * during slot synchronization because we need to retain the same
+ * values as the remote slot.
+ */
+ if (two_phase && !IsSyncingReplicationSlots())
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable both \"%s\" and \"%s\" options during replication slot creation",
+ "failover", "two_phase"));
}
/*
@@ -848,6 +868,23 @@ ReplicationSlotAlter(const char *name, bool failover)
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot enable failover for a temporary replication slot"));
+ /*
+ * Do not allow users to enable failover for a two_phase enabled slot
+ * where slot's restart_lsn is less than two_phase_at. In such cases,
+ * there is a risk that transactions prepared before two_phase_at exist
+ * and would be skipped during decoding.
+ *
+ * See comments atop the similar check in ReplicationSlotCreate() for a
+ * detailed reason.
+ */
+ if (failover && MyReplicationSlot->data.two_phase &&
+ MyReplicationSlot->data.restart_lsn < MyReplicationSlot->data.two_phase_at)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot enable failover for a two-phase enabled replication slot"),
+ errdetail("The slot need to consume change upto %X/%X to enable failover.",
+ LSN_FORMAT_ARGS(MyReplicationSlot->data.two_phase_at)));
+
if (MyReplicationSlot->data.failover != failover)
{
SpinLockAcquire(&MyReplicationSlot->mutex);
diff --git a/src/bin/pg_upgrade/t/003_logical_slots.pl b/src/bin/pg_upgrade/t/003_logical_slots.pl
index 0a2483d3dfc..e329cc609ce 100644
--- a/src/bin/pg_upgrade/t/003_logical_slots.pl
+++ b/src/bin/pg_upgrade/t/003_logical_slots.pl
@@ -173,7 +173,7 @@ $sub->start;
$sub->safe_psql(
'postgres', qq[
CREATE TABLE tbl (a int);
- CREATE SUBSCRIPTION regress_sub CONNECTION '$old_connstr' PUBLICATION regress_pub WITH (two_phase = 'true', failover = 'true')
+ CREATE SUBSCRIPTION regress_sub CONNECTION '$old_connstr' PUBLICATION regress_pub WITH (two_phase = 'true')
]);
$sub->wait_for_subscription_sync($oldpub, 'regress_sub');
@@ -193,8 +193,8 @@ command_ok([@pg_upgrade_cmd], 'run of pg_upgrade of old cluster');
# Check that the slot 'regress_sub' has migrated to the new cluster
$newpub->start;
my $result = $newpub->safe_psql('postgres',
- "SELECT slot_name, two_phase, failover FROM pg_replication_slots");
-is($result, qq(regress_sub|t|t), 'check the slot exists on new cluster');
+ "SELECT slot_name, two_phase FROM pg_replication_slots");
+is($result, qq(regress_sub|t), 'check the slot exists on new cluster');
# Update the connection
my $new_connstr = $newpub->connstr . ' dbname=postgres';
diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl
index 823857bb329..22f3779be3c 100644
--- a/src/test/recovery/t/040_standby_failover_slots_sync.pl
+++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl
@@ -25,6 +25,8 @@ $publisher->init(
$publisher->append_conf('postgresql.conf', 'autovacuum = off');
$publisher->start;
+$publisher->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
+
$publisher->safe_psql('postgres',
"CREATE PUBLICATION regress_mypub FOR ALL TABLES;");
@@ -42,6 +44,8 @@ my $slot_creation_time_on_primary = $publisher->safe_psql(
SELECT current_timestamp;
]);
+$subscriber1->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
+
# Create a subscription that enables failover.
$subscriber1->safe_psql('postgres',
"CREATE SUBSCRIPTION regress_mysub1 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (slot_name = lsub1_slot, copy_data = false, failover = true, enabled = false);"
@@ -98,6 +102,123 @@ my ($result, $stdout, $stderr) = $subscriber1->psql('postgres',
ok( $stderr =~ /ERROR: cannot set failover for enabled subscription/,
"altering failover is not allowed for enabled subscription");
+##################################################
+# Test that the failover option can be enabled for a two_phase enabled
+# subscription only through Alter Subscription (failover=true)
+##################################################
+
+# Create a subscription with two_phase enabled
+$subscriber1->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_mysub2 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (slot_name = lsub2_slot, two_phase = true);"
+);
+
+# Wait for initial table sync to finish
+$subscriber1->wait_for_subscription_sync($publisher, 'regress_mysub2');
+
+# Also wait for two-phase to be enabled
+$subscriber1->poll_query_until(
+ 'postgres', qq[
+ SELECT count(1) = 0 FROM pg_subscription WHERE subname = 'regress_mysub2' and subtwophasestate NOT IN ('e');]
+) or die "Timed out while waiting for subscriber to enable twophase";
+
+# Try to enable the failover for the subscription, should give error
+($result, $stdout, $stderr) = $subscriber1->psql(
+ 'postgres',
+ "ALTER SUBSCRIPTION regress_mysub2 DISABLE;
+ ALTER SUBSCRIPTION regress_mysub2 SET (failover = true);");
+ok( $stderr =~
+ qr/ERROR: could not alter replication slot "lsub2_slot": ERROR: cannot enable failover for a two-phase enabled replication slot/,
+ "failover can't be enabled if restart_lsn < two_phase_at on a two_phase subscription."
+);
+
+$subscriber1->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_mysub2 ENABLE;");
+
+# Advance the slot's restart_lsn to allow enabling the failover option
+# on a two_phase-enabled subscription using ALTER SUBSCRIPTION.
+$publisher->safe_psql(
+ 'postgres', qq(
+ BEGIN;
+ SELECT txid_current();
+ SELECT pg_log_standby_snapshot();
+ COMMIT;
+ BEGIN;
+ SELECT txid_current();
+ SELECT pg_log_standby_snapshot();
+ COMMIT;
+));
+
+# Alter subscription to enable failover
+($result, $stdout, $stderr) = $subscriber1->psql(
+ 'postgres',
+ "ALTER SUBSCRIPTION regress_mysub2 DISABLE;
+ ALTER SUBSCRIPTION regress_mysub2 SET (failover = true);");
+
+# Confirm that the failover flag on the slot has been turned on
+is( $publisher->safe_psql(
+ 'postgres',
+ q{SELECT failover from pg_replication_slots WHERE slot_name = 'lsub2_slot';}
+ ),
+ "t",
+ 'logical slot has failover true on the publisher');
+
+# Drop the subscription
+$subscriber1->safe_psql('postgres', "DROP SUBSCRIPTION regress_mysub2;");
+
+# Test that SQL API disallows slot creation with both two_phase and failover enabled
+($result, $stdout, $stderr) = $publisher->psql('postgres',
+ "SELECT pg_create_logical_replication_slot('regress_mysub3', 'pgoutput', false, true, true);"
+);
+ok( $stderr =~
+ /ERROR: cannot enable both "failover" and "two_phase" options during replication slot creation/,
+ "cannot create slot with both two_phase and failover enabled");
+
+# Test that the failover option cannot be set for a subscription with
+# two_phase in pending state.
+
+# Create a subscription with two_phase enabled, but in pending state
+$subscriber1->psql('postgres',
+ "CREATE SUBSCRIPTION regress_mysub3 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (enabled = false, two_phase = true);"
+);
+
+# Enable failover for the subscription with two_phase in pending state
+($result, $stdout, $stderr) = $subscriber1->psql('postgres',
+ "ALTER SUBSCRIPTION regress_mysub3 SET (failover = true)");
+ok( $stderr =~
+ /ERROR: cannot enable failover for a subscription with a pending two_phase state
+.*HINT: The "failover" option can be enabled after "two_phase" state is ready./,
+ "enabling failover is not allowed for a two_phase pending subscription");
+
+# Drop the subscription
+$subscriber1->safe_psql('postgres', "DROP SUBSCRIPTION regress_mysub3;");
+
+# Test that subscription creation with two_phase=true results in error when
+# using a slot with failover set to true.
+
+# Create a slot with failover enabled
+$publisher->psql('postgres',
+ "SELECT pg_create_logical_replication_slot('regress_mysub4', 'pgoutput', false, false, true);"
+);
+
+my $logoffset = -s $subscriber1->logfile;
+
+# Create a subscription with two_phase enabled
+$subscriber1->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_mysub4 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (create_slot = false, two_phase = true);"
+);
+
+ok( $subscriber1->wait_for_log(
+ qr/cannot enable two-phase decoding for failover enabled slot "regress_mysub4"/,
+ $logoffset),
+ "creating subscription with two_phase=true is not allowed when the slot has failover set to true"
+);
+# Drop the subscription
+$subscriber1->safe_psql('postgres', "DROP SUBSCRIPTION regress_mysub4;");
+
+# Clean up the tables created
+$publisher->safe_psql('postgres', "DROP TABLE tab_full;");
+$subscriber1->safe_psql('postgres', "DROP TABLE tab_full;");
+
##################################################
# Test that pg_sync_replication_slots() cannot be executed on a non-standby server.
##################################################
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 0f2a25cdc19..66b1f5aa08f 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -479,6 +479,10 @@ COMMIT;
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
DROP SUBSCRIPTION regress_testsub;
RESET SESSION AUTHORIZATION;
+-- fail - cannot enable two_phase and failover together
+CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, two_phase = true, failover = true);
+ERROR: cannot enable both "failover" and "two_phase" options at CREATE SUBSCRIPTION
+HINT: The "failover" option can be enabled after "two_phase" state is ready using ALTER SUBSCRIPTION.
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user2;
DROP ROLE regress_subscription_user3;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 3e5ba4cb8c6..47fc1e5329b 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -342,6 +342,10 @@ ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
DROP SUBSCRIPTION regress_testsub;
RESET SESSION AUTHORIZATION;
+
+-- fail - cannot enable two_phase and failover together
+CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, two_phase = true, failover = true);
+
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user2;
DROP ROLE regress_subscription_user3;
--
2.34.1
