master_v7-amit.diff.txt

text/plain

Filename: master_v7-amit.diff.txt
Type: text/plain
Part: 0
Message: Re: Restrict copying of invalidated replication slots
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 999c14c920d..215b797664f 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -694,22 +694,7 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 		plugin = NameStr(*(PG_GETARG_NAME(3)));
 	}
 
-	/*
-	 * Create new slot and acquire it
-	 *
-	 * After slot is created there can be a race condition with function
-	 * InvalidateObsoleteReplicationSlots, when the copied slot appear before
-	 * source slot in array ReplicationSlotCtl->replication_slots.
-	 *
-	 * If just after slot creation, the source slot is invalidated due to
-	 * some operation. The execution of InvalidateObsoleteReplicationSlots will
-	 * wait for this function to release the copied slot. Then the source slot
-	 * will be invalidated. So, the copying of source slot is successful in this
-	 * case.
-	 *
-	 * The wal_status of copied slot is set to lost immediately and hence will
-	 * not have any harm.
-	 */
+	/* Create new slot and acquire it */
 	if (logical_slot)
 	{
 		/*
@@ -801,7 +786,14 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 							NameStr(*src_name)),
 					 errhint("Retry when the source replication slot's confirmed_flush_lsn is valid.")));
 
-		/* Check if source slot became invalidated during the copy operation */
+		/*
+		 * Copying an invalid slot doesn't make sense. Note that the source
+		 * slot can become invalid after we creat the new slot and copy the
+		 * data of source slot. This is possible because the operations in
+		 * InvalidateObsoleteReplicationSlots() are not serialized with this
+		 * function. Even though we can't detect such a case here, the copied
+		 * slot will become invalid in the next checkpoint cycle.
+		 */
 		if (second_slot_contents.data.invalidated != RS_INVAL_NONE)
 			ereport(ERROR,
 					errmsg("cannot copy replication slot \"%s\"",