v82_0001_amit.patch.txt

text/plain

Filename: v82_0001_amit.patch.txt
Type: text/plain
Part: 0
Message: Re: Synchronizing slots from primary to standby
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index b8b5bd61b4..0d5ca29c18 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -13,14 +13,15 @@
  * the slots on the standby and synchronize them. This is done by a call to SQL
  * function pg_sync_replication_slots.
  *
- * If on the physical standby, the restart_lsn and/or local catalog_xmin is
- * ahead of those on the remote then we cannot create the local standby slot
- * in sync with the primary server because that would mean moving the local
- * slot backwards and the standby might not have WALs retained for old LSN.
- * In this case, the slot will be marked as RS_TEMPORARY. Once the primary
- * server catches up, the slot will be marked as RS_PERSISTENT (which
- * means sync-ready) after which we can call pg_sync_replication_slots()
- * periodically to perform syncs.
+ * If on physical standby, the WAL corresponding to the remote's restart_lsn
+ * is not available or the remote's catalog_xmin precedes the oldest xid for which
+ * it is guaranteed that rows wouldn't have been removed then we cannot create
+ * the local standby slot because that would mean moving the local slot
+ * backward and decoding won't be possible via such a slot. In this case, the
+ * slot will be marked as RS_TEMPORARY. Once the primary server catches up,
+ * the slot will be marked as RS_PERSISTENT (which means sync-ready) after
+ * which we can call pg_sync_replication_slots() periodically to perform
+ * syncs.
  *
  * Any standby synchronized slots will be dropped if they no longer need
  * to be synchronized. See comment atop drop_local_obsolete_slots() for more
@@ -60,15 +61,10 @@
 #include "utils/timeout.h"
 #include "utils/varlena.h"
 
-/*
- * Struct for sharing information to control slot synchronization.
- *
- * The 'syncing' flag should be set to true in any process that is syncing
- * slots to prevent concurrent slot sync, which could lead to errors and slot
- * info overwrite.
- */
+/* Struct for sharing information to control slot synchronization. */
 typedef struct SlotSyncCtxStruct
 {
+	/* prevents concurrent slot syncs to avoid slot overwrites */
 	bool		syncing;
 	slock_t		mutex;
 } SlotSyncCtxStruct;
@@ -933,8 +929,8 @@ SlotSyncInitialize(void)
 }
 
 /*
- * Synchronize the replication slots with failover enabled using the
- * specified primary server connection.
+ * Synchronize the failover enabled replication slots using the specified
+ * primary server connection.
  */
 void
 SyncReplicationSlots(WalReceiverConn *wrconn)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index c443e949b2..94447f6228 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -266,10 +266,10 @@ ReplicationSlotCreate(const char *name, bool db_specific,
 	ReplicationSlotValidateName(name, ERROR);
 
 	/*
-	 * Do not allow users to create the slots with failover enabled on the
-	 * standby as we do not support sync to the cascading standby.
+	 * Do not allow users to create the failover enabled slots on the standby
+	 * as we do not support sync to the cascading standby.
 	 *
-	 * However, slots with failover enabled can be created during slot
+	 * However, failover enabled slots can be created during slot
 	 * synchronization because we need to retain the same values as the remote
 	 * slot.
 	 */
@@ -736,8 +736,8 @@ ReplicationSlotAlter(const char *name, bool failover)
 					errdetail("This slot is being synced from the primary server."));
 
 		/*
-		 * Do not allow users to alter slots to enable failover on the standby
-		 * as we do not support sync to the cascading standby.
+		 * Do not allow users to enable failover on the standby as we do not
+		 * support sync to the cascading standby.
 		 */
 		if (failover)
 			ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index cf528db17a..323ce9c1fc 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -794,13 +794,13 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 		 * hence pass find_startpoint false.  confirmed_flush will be set
 		 * below, by copying from the source slot.
 		 *
-		 * To avoid potential issues with the slot synchronization when the
-		 * restart_lsn of a replication slot goes backwards, we set the
-		 * failover option to false here. This situation occurs when a slot on
+		 * To avoid potential issues with the slot synchronization where the
+		 * restart_lsn of a replication slot can go backward, we set the
+		 * failover option to false here.  This situation occurs when a slot on
 		 * the primary server is dropped and immediately replaced with a new
 		 * slot of the same name, created by copying from another existing
-		 * slot. However, the slot synchronization will only observe the
-		 * restart_lsn of the same slot going backwards.
+		 * slot.  However, the slot synchronization will only observe the
+		 * restart_lsn of the same slot going backward.
 		 */
 		create_logical_replication_slot(NameStr(*dst_name),
 										plugin,
@@ -955,8 +955,8 @@ pg_copy_physical_replication_slot_b(PG_FUNCTION_ARGS)
 }
 
 /*
- * Synchronize replication slots with failover enabled to a standby server from
- * the primary server.
+ * Synchronize failover enabled replication slots with to a standby server
+ * from the primary server.
  */
 Datum
 pg_sync_replication_slots(PG_FUNCTION_ARGS)