On Tue, Jun 10, 2025 at 2:29 PM shveta malik <shveta.ma...@gmail.com> wrote:
>
> On Fri, Jun 6, 2025 at 12:37 PM Nisha Moond <nisha.moond...@gmail.com> wrote:
> >
> > Attached v18 patch.
> > - patch-001: modified error messages as suggested above.
> > - patch-002: improved pg_dump docs as per Shveta's off-list suggestions.
> >
>
> Thanks for the patches. Please find few comments:
>
Thanks for the review.
> 1)
> + * However, we allow this combination in binary upgrade mode, where
> + * pg_upgrade guarantees that all slot changes are consumed and no
> + * prepared transactions exist.
> + */
>
> Can we please mention on which system pg_upgrade ensures that no
> prepared txn exists. Is it at the source system or target system or
> both? The similar comment is there at other place too, please update
> that as well.
>
pg_upgrade checks for prepared transactions on both source and target
systems. I've updated the respective comments.
~~~
Attached v19 patches addressing all the comments in [1] and [2].
[1]
https://www.postgresql.org/message-id/CAJpy0uCGE0fi4oMb9A_L27C-fJq%2BVpSO9ZbcVxW5s5F3%2BsX0Lw%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAFPTHDYXjq_%3Dr2ss6YMQBr%3DwbSfkH%3DuGJFrJzVpaW0eqPhy0RQ%40mail.gmail.com
--
Thanks,
Nisha
From b6c1032a504f699256327160e5b9dfd99ae4f962 Mon Sep 17 00:00:00 2001
From: Nisha Moond <nisha.moond412@gmail.com>
Date: Tue, 3 Jun 2025 10:42:53 +0530
Subject: [PATCH v19 1/2] PG17 Approach 3 Fix slot synchronization for
two-phase enabled 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 subscription's failover to true when two-phase state is 'pending'.
User can try altering failover again when two-phase state is moved to 'enabled'.
---
contrib/test_decoding/expected/slot.out | 2 +
contrib/test_decoding/sql/slot.sql | 1 +
doc/src/sgml/func.sgml | 13 ++
doc/src/sgml/protocol.sgml | 17 +++
doc/src/sgml/ref/alter_subscription.sgml | 10 ++
doc/src/sgml/ref/create_subscription.sgml | 17 +++
src/backend/commands/subscriptioncmds.c | 28 ++++
src/backend/replication/logical/logical.c | 11 ++
src/backend/replication/logical/slotsync.c | 10 ++
src/backend/replication/slot.c | 30 +++++
src/bin/pg_upgrade/t/003_logical_slots.pl | 32 ++++-
.../t/040_standby_failover_slots_sync.pl | 124 ++++++++++++++++++
src/test/regress/expected/subscription.out | 4 +
src/test/regress/sql/subscription.sql | 4 +
14 files changed, 302 insertions(+), 1 deletion(-)
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 697c1a02891..45504cf14d5 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -29076,6 +29076,19 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
failover. A call to this function has the same effect as
the replication protocol command
<literal>CREATE_REPLICATION_SLOT ... LOGICAL</literal>.
+
+ <note>
+ <para>
+ 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 either using
+ <xref linkend="protocol-replication-alter-replication-slot"/> or via
+ <xref linkend="sql-altersubscription"/> if a subscription is using
+ this slot.
+ </para>
+ </note>
</para></entry>
</row>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 0396d3a9e98..2c2d66fad35 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2062,6 +2062,15 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
<literal>PREPARE TRANSACTION</literal> time.
The default is false.
</para>
+
+ <para>
+ 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 the changes up to the point where two-phase decoding
+ was enabled.
+ </para>
</listitem>
</varlistentry>
@@ -2103,6 +2112,14 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
so that logical replication can be resumed after failover.
The default is false.
</para>
+
+ <para>
+ 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 the changes up to the
+ point where two-phase decoding was enabled.
+ </para>
</listitem>
</varlistentry>
</variablelist>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 2ccbf5e4897..ec3fc44e4e5 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -257,6 +257,16 @@ 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 subscription's two-phase is in the pending state, setting
+ <literal>failover</literal> to true is not permitted. Once the two-phase
+ state is enabled, <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
+ can be set to true only after the slot has consumed all the changes up to the
+ point where two-phase decoding was enabled. See column <structfield>subtwophasestate</structfield>
+ of <link linkend="catalog-pg-subscription"><structname>pg_subscription</structname></link>
+ to know the actual two-phase state.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index c9c8dd440dc..55b20893e80 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -354,6 +354,14 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
to know the actual two-phase state.
</para>
+ <para>
+ This option cannot be enabled together with
+ <literal>failover</literal> when creating a subscription. However, a
+ <literal>two_phase</literal> enabled subscription can later have
+ <literal>failover</literal> set to true using
+ <xref linkend="sql-altersubscription"/>, once the slot has consumed
+ all the changes up to the point where two-phase decoding was enabled.
+ </para>
</listitem>
</varlistentry>
@@ -426,6 +434,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>
+ This option cannot be enabled together with
+ <literal>two_phase</literal> when creating a subscription. However, a
+ <literal>two_phase</literal> enabled subscription can later have
+ <literal>failover</literal> set to true using
+ <xref linkend="sql-altersubscription"/>, once the slot has consumed
+ all the changes 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..a5c33f879d3 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 slotsync.c for details.
+ *
+ * However, we allow this combination in binary upgrade mode, where
+ * pg_upgrade guarantees that all slot changes are consumed and no
+ * prepared transactions exist on either the old or new cluster.
+ */
+ if (opts.twophase && opts.failover && !IsBinaryUpgrade)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable both \"%s\" and \"%s\" options at \"CREATE SUBSCRIPTION\"",
+ "failover", "two_phase"),
+ errhint("Use \"ALTER SUBSCRIPTION ... SET (failover = true)\" to enable failover after two-phase state is ready"));
+
/*
* If built with appropriate switch, whine when regression-testing
* conventions for subscription names are violated.
@@ -1245,6 +1260,19 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
errmsg("cannot set %s for enabled subscription",
"failover")));
+ /*
+ * Do not allow users to enable the failover until
+ * two_phase state reaches READY state. See comments atop
+ * slotsync.c for details.
+ */
+ 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 two-phase state is ready.",
+ "failover"));
+
/*
* 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 97b6aa899ee..1beb7c2eb56 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -613,6 +613,17 @@ 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 slotsync.c for details.
+ */
+ 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/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index 73fe3f56af2..011c244ac35 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -42,6 +42,16 @@
* Any standby synchronized slots will be dropped if they no longer need
* to be synchronized. See comment atop drop_local_obsolete_slots() for more
* details.
+ *
+ * Regarding two-phase enabled slots, we need to note that the two_phase_at
+ * field of a slot is not synchronized. In the absence of two_phase_at,
+ * logical decoding could incorrectly identify prepared transactions as
+ * having been replicated to the subscriber after promotion, resulting in
+ * their omission. Consequently, both two_phase and failover features
+ * cannot be simultaneously enabled during slot creation. Failover can only
+ * be enabled once we can ensure that no transactions were prepared prior
+ * to two_phase_at on the primary. These restrictions are enforced through
+ * checks in ReplicationSlotCreate() and ReplicationSlotAlter().
*---------------------------------------------------------------------------
*/
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index a1d4768623f..8e0748d2528 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -343,6 +343,23 @@ 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.
+ * Please see the comments atop slotsync.c for details.
+ *
+ * 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. This is also allowed in binary upgrade
+ * mode, where pg_upgrade guarantees that all slot changes are
+ * consumed and no prepared transactions exist on either the old or
+ * new cluster.
+ */
+ if (two_phase && !IsSyncingReplicationSlots() && !IsBinaryUpgrade)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable both \"%s\" and \"%s\" options during replication slot creation",
+ "failover", "two_phase"));
}
/*
@@ -848,6 +865,19 @@ 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 if
+ * there are potentially un-decoded transactions that are prepared before
+ * two_phase_at. See comments atop slotsync.c for details.
+ */
+ 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("could not enable failover for a two-phase enabled replication slot due to unconsumed changes"),
+ errdetail("The slot needs to consume change up to %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..76b521b51bd 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');
@@ -183,8 +183,38 @@ my $twophase_query =
$sub->poll_query_until('postgres', $twophase_query)
or die "Timed out while waiting for subscriber to enable twophase";
+# Advance the slot's restart_lsn to allow enabling the failover option
+# on a two_phase-enabled subscription using ALTER SUBSCRIPTION.
+$oldpub->safe_psql(
+ 'postgres', qq(
+ BEGIN;
+ SELECT txid_current();
+ SELECT pg_log_standby_snapshot();
+ COMMIT;
+ BEGIN;
+ SELECT txid_current();
+ SELECT pg_log_standby_snapshot();
+ COMMIT;
+));
+
+# Wait for the subscription to be in sync
+$sub->wait_for_subscription_sync($oldpub, 'regress_sub');
+
# 2. Temporarily disable the subscription
$sub->safe_psql('postgres', "ALTER SUBSCRIPTION regress_sub DISABLE");
+
+# Alter subscription to enable failover
+$sub->psql('postgres',
+ "ALTER SUBSCRIPTION regress_sub SET (failover = true);");
+
+# Confirm that the failover flag on the slot has been turned on
+is( $oldpub->safe_psql(
+ 'postgres', q{
+ SELECT failover from pg_replication_slots WHERE slot_name = 'regress_sub';}
+ ),
+ "t",
+ 'logical slot has failover true on the publisher');
+
$oldpub->stop;
# pg_upgrade should be successful
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..0db7e6866d1 100644
--- a/src/test/recovery/t/040_standby_failover_slots_sync.pl
+++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl
@@ -98,6 +98,130 @@ 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 table on both publisher and subscriber to complete subscription
+# initialization, allowing two_phase to transition from 'pending' to 'enabled'.
+$publisher->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
+$subscriber1->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
+
+# Test that the failover option cannot be set for a slot with
+# restart_lsn < two_phase_at
+
+# Create an logical replication slot with two_phase enabled
+$publisher->psql('postgres',
+ "SELECT pg_create_logical_replication_slot('logical_slot', 'pgoutput', false, true, false);"
+);
+
+# Try to enable failover of the slot
+($result, $stdout, $stderr) = $publisher->psql(
+ 'postgres',
+ qq[ALTER_REPLICATION_SLOT logical_slot (failover);],
+ replication => 'database');
+ok( $stderr =~
+ qr/ERROR: could not enable failover for a two-phase enabled replication slot due to unconsumed changes/,
+ "failover can't be enabled if restart_lsn < two_phase_at on a two_phase subscription."
+);
+
+# Drop the logical_slot
+$publisher->psql('postgres',
+ "SELECT pg_drop_replication_slot('logical_slot');");
+
+# Test that the failover option can be set for a two_phase enabled
+# subscription using ALTER SUBSCRIPTION.
+
+# Create a subscription with two_phase enabled
+$subscriber1->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_mysub2 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (enabled = false, two_phase = true);"
+);
+
+# Try to enable failover for the subscription with two_phase in pending state
+($result, $stdout, $stderr) = $subscriber1->psql('postgres',
+ "ALTER SUBSCRIPTION regress_mysub2 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");
+
+# Enable the subscription
+$subscriber1->safe_psql('postgres',
+ "ALTER SUBSCRIPTION regress_mysub2 ENABLE;");
+
+# Wait for the subscription to be in sync
+$subscriber1->wait_for_subscription_sync($publisher, 'regress_mysub2');
+
+# Also wait for two-phase to be enabled
+ok( $subscriber1->poll_query_until(
+ 'postgres',
+ qq[SELECT subtwophasestate FROM pg_subscription WHERE subname = 'regress_mysub2';],
+ 'e',),
+ "Timed out while waiting for subscriber to enable twophase");
+
+# 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;
+));
+
+# Wait for the subscription to be in sync
+$subscriber1->wait_for_subscription_sync($publisher, 'regress_mysub2');
+
+# Alter subscription to enable failover
+$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 = 'regress_mysub2';}
+ ),
+ "t",
+ 'logical slot has failover true on the publisher');
+
+# Drop the subscription
+$subscriber1->safe_psql('postgres', "DROP SUBSCRIPTION regress_mysub2;");
+
+# 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_mysub3', 'pgoutput', false, false, true);"
+);
+
+my $logoffset = -s $subscriber1->logfile;
+
+# Create a subscription with two_phase enabled
+$subscriber1->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_mysub3 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_mysub3"/,
+ $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_mysub3;");
+
+# 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..9f4daf47901 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: Use "ALTER SUBSCRIPTION ... SET (failover = true)" to enable failover after two-phase state is ready
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
From b0e26e90d99ebfb2602a7417c2c221df84767d57 Mon Sep 17 00:00:00 2001
From: Nisha Moond <nisha.moond412@gmail.com>
Date: Tue, 3 Jun 2025 10:48:00 +0530
Subject: [PATCH v19 2/2] Don't dump failover=true with two_phase in CREATE
SUBSCRIPTION
When dumping logical replication subscriptions where both two_phase and
failover are enabled, generate the CREATE SUBSCRIPTION commands with
only two_phase = true option, omitting the failover=true option.
That way, the dump can be restored without encountering the error caused
by enabling both failover and two_phase simultaneously during CREATE
SUBSCRIPTION. After the restore, users can enable failover using ALTER
SUBSCRIPTION.
---
doc/src/sgml/ref/pg_dump.sgml | 13 +++++++++++++
src/bin/pg_dump/pg_dump.c | 12 +++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index cfc74ca6d69..6c133c5e12a 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1633,6 +1633,19 @@ CREATE DATABASE foo WITH TEMPLATE template0;
had been originally created with <literal>two_phase = true</literal> option.
</para>
+ <para>
+ When dumping logical replication subscriptions where both
+ <link linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link>
+ and <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
+ are enabled, <application>pg_dump</application> will generate <command>CREATE
+ SUBSCRIPTION</command> commands with only <literal>two_phase = true</literal>
+ option, omitting the <literal>failover=true</literal> option. That way,
+ the dump can be restored without encountering the error caused by enabling
+ both failover and two_phase simultaneously during <xref linkend="sql-createsubscription"/>.
+ After the restore, users can enable <literal>failover</literal> using
+ <xref linkend="sql-altersubscription"/>.
+ </para>
+
<para>
It is generally recommended to use the <option>-X</option>
(<option>--no-psqlrc</option>) option when restoring a database from a
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9ed1a856fa3..cb5f4b0812b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5129,6 +5129,7 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
int npubnames = 0;
int i;
char two_phase_disabled[] = {LOGICALREP_TWOPHASE_STATE_DISABLED, '\0'};
+ bool set_failover = true;
/* Do nothing in data-only dump */
if (dopt->dataOnly)
@@ -5174,8 +5175,17 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
appendPQExpBufferStr(query, ", streaming = parallel");
if (strcmp(subinfo->subtwophasestate, two_phase_disabled) != 0)
+ {
appendPQExpBufferStr(query, ", two_phase = on");
+ /*
+ * The failover option cannot be set together with two_phase during
+ * CREATE SUBSCRIPTION. It can be enabled later using ALTER
+ * SUBSCRIPTION after the subscription is restored.
+ */
+ set_failover = false;
+ }
+
if (strcmp(subinfo->subdisableonerr, "t") == 0)
appendPQExpBufferStr(query, ", disable_on_error = true");
@@ -5185,7 +5195,7 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
if (strcmp(subinfo->subrunasowner, "t") == 0)
appendPQExpBufferStr(query, ", run_as_owner = true");
- if (strcmp(subinfo->subfailover, "t") == 0)
+ if (set_failover && strcmp(subinfo->subfailover, "t") == 0)
appendPQExpBufferStr(query, ", failover = true");
if (strcmp(subinfo->subsynccommit, "off") != 0)
--
2.34.1
