v3-0002-amit.1.patch.txt

text/plain

Filename: v3-0002-amit.1.patch.txt
Type: text/plain
Part: 0
Message: Re: Fix slot synchronization with two_phase decoding enabled
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 6db3c9347fa..09c5f0ef685 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -649,10 +649,11 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
 				 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 failover and two_phase option together.
+	 * Do not allow users to enable the failover and two_phase options
+	 * together.
 	 *
-	 * See comments atop the similar check in ReplicationSlotCreate() for
-	 * detailed reasons.
+	 * See comments atop the similar check in ReplicationSlotCreate() for a
+	 * detailed reason.
 	 */
 	if (opts.twophase && opts.failover)
 		ereport(ERROR,
@@ -1258,11 +1259,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
 										"failover")));
 
 					/*
-					 * Do not allow users to enable failover and two_phase
-					 * option together.
+					 * Do not allow users to enable the failover and two_phase
+					 * options together.
 					 *
 					 * See comments atop the similar check in
-					 * ReplicationSlotCreate() for detailed reasons.
+					 * ReplicationSlotCreate() for a detailed reason.
 					 */
 					if (sub->twophasestate != LOGICALREP_TWOPHASE_STATE_DISABLED &&
 						opts.failover)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index f40028bd941..d251ce95fe6 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -345,14 +345,14 @@ ReplicationSlotCreate(const char *name, bool db_specific,
 					errmsg("cannot enable failover for a temporary replication slot"));
 
 		/*
-		 * Do not allow users to enable failover for slots that enable
-		 * two-phase decoding.
+		 * 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 this field, the logical decoding might
+		 * 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, causing them to be skipped.
+		 * the subscriber after promotion of standby server, causing them to be
+		 * skipped.
 		 */
 		if (two_phase)
 			ereport(ERROR,
@@ -865,11 +865,10 @@ ReplicationSlotAlter(const char *name, bool failover)
 				errmsg("cannot enable failover for a temporary replication slot"));
 
 	/*
-	 * Do not allow users to enable failover for slots that enable two-phase
-	 * decoding.
+	 * Do not allow users to enable failover for a two_phase enabled slot.
 	 *
-	 * See comments atop the similar check in ReplicationSlotCreate() for
-	 * detailed reasons.
+	 * See comments atop the similar check in ReplicationSlotCreate() for a
+	 * detailed reason.
 	 */
 	if (failover && MyReplicationSlot->data.two_phase)
 		ereport(ERROR,