Fix assertion failure during decoding from synced slots.

Amit Kapila · Amit Kapila · commit 3ff2a1f0c9e8 · 2025-04-29T12:52:05.000+05:30
The slot synchronization skips updating the confirmed_flush LSN of the local slot if the local slot has a newer catalog_xmin or restart_lsn, but still allows updating the two_phase and two_phase_at fields of the slot. This opens up a window for the prepared transactions between old confirmed_flush LSN and two_phase_at to unexpectedly get decoded and sent to the downstream after promotion. Then, while decoding the commit prepared the assert will fail, which expects that the prepare hasn't been sent to the downstream. The fix is to skip updating the other slot fields when we are skipping to update the confirmed_flush LSN of the slot. We didn't backpatch this commit as two_phase_at was not synced in back branches, which means prepared transactions won't be unexpectedly sent to downstream. We discovered this problem while analyzing BF failure reported in the discussion link. Reliably reproducing this issue without a debugger is difficult. Given its rarity, adding specific injection point to test it doesn't seem worthwhile, so we won't be adding a dedicated test case. Author: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/OS0PR01MB5716B44052000EB91EFAE60E94BC2@OS0PR01MB5716.jpnprd01.prod.outlook.com
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
@@ -196,14 +196,14 @@ update_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid,
 		 * restart_lsn or the initial xmin_horizon computed for the local slot
 		 * is ahead of the remote slot.
 		 *
-		 * If the slot is persistent, restart_lsn of the synced slot could
-		 * still be ahead of the remote slot. Since we use slot advance
-		 * functionality to keep snapbuild/slot updated, it is possible that
-		 * the restart_lsn is advanced to a later position than it has on the
-		 * primary. This can happen when slot advancing machinery finds
-		 * running xacts record after reaching the consistent state at a later
-		 * point than the primary where it serializes the snapshot and updates
-		 * the restart_lsn.
+		 * If the slot is persistent, both restart_lsn and catalog_xmin of the
+		 * synced slot could still be ahead of the remote slot. Since we use
+		 * slot advance functionality to keep snapbuild/slot updated, it is
+		 * possible that the restart_lsn and catalog_xmin are advanced to a
+		 * later position than it has on the primary. This can happen when
+		 * slot advancing machinery finds running xacts record after reaching
+		 * the consistent state at a later point than the primary where it
+		 * serializes the snapshot and updates the restart_lsn.
 		 *
 		 * We LOG the message if the slot is temporary as it can help the user
 		 * to understand why the slot is not sync-ready. In the case of a
@@ -221,16 +221,25 @@ update_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid,
 
 		if (remote_slot_precedes)
 			*remote_slot_precedes = true;
+
+		/*
+		 * Skip updating the configuration. This is required to avoid syncing
+		 * two_phase_at without syncing confirmed_lsn. Otherwise, the prepared
+		 * transaction between old confirmed_lsn and two_phase_at will
+		 * unexpectedly get decoded and sent to the downstream after
+		 * promotion. See comments in ReorderBufferFinishPrepared.
+		 */
+		return false;
 	}
 
 	/*
 	 * Attempt to sync LSNs and xmins only if remote slot is ahead of local
 	 * slot.
 	 */
-	else if (remote_slot->confirmed_lsn > slot->data.confirmed_flush ||
-			 remote_slot->restart_lsn > slot->data.restart_lsn ||
-			 TransactionIdFollows(remote_slot->catalog_xmin,
-								  slot->data.catalog_xmin))
+	if (remote_slot->confirmed_lsn > slot->data.confirmed_flush ||
+		remote_slot->restart_lsn > slot->data.restart_lsn ||
+		TransactionIdFollows(remote_slot->catalog_xmin,
+							 slot->data.catalog_xmin))
 	{
 		/*
 		 * We can't directly copy the remote slot's LSN or xmin unless there
@@ -294,6 +303,12 @@ update_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid,
 		SpinLockRelease(&slot->mutex);
 
 		updated_config = true;
+
+		/*
+		 * Ensure that there is no risk of sending prepared transactions
+		 * unexpectedly after the promotion.
+		 */
+		Assert(slot->data.two_phase_at <= slot->data.confirmed_flush);
 	}
 
 	/*
