Merge pull request #1504 from citusdata/recovery_record_entropy

Add transaction number to 2PC identifiers

src/backend/distributed/transaction/backend_data.c

@@ -145,7 +145,7 @@ get_current_transaction_id(PG_FUNCTION_ARGS)
 		ereport(ERROR, (errmsg("backend is not ready for distributed transactions")));
 	}
 
-	distributedTransctionId = GetCurrentDistributedTransctionId();
+	distributedTransctionId = GetCurrentDistributedTransactionId();
 
 	memset(values, 0, sizeof(values));
 	memset(isNulls, false, sizeof(isNulls));
@@ -434,11 +434,11 @@ UnSetDistributedTransactionId(void)
 
 
 /*
- * GetCurrentDistributedTransctionId reads the backend's distributed transaction id and
+ * GetCurrentDistributedTransactionId reads the backend's distributed transaction id and
  * returns a copy of it.
  */
 DistributedTransactionId *
-GetCurrentDistributedTransctionId(void)
+GetCurrentDistributedTransactionId(void)
 {
 	DistributedTransactionId *currentDistributedTransactionId =
 		(DistributedTransactionId *) palloc(sizeof(DistributedTransactionId));
@@ -487,3 +487,17 @@ AssignDistributedTransactionId(void)
 
 	SpinLockRelease(&MyBackendData->mutex);
 }
+
+
+/*
+ * CurrentDistributedTransactionNumber returns the transaction number of the
+ * current distributed transaction. The caller must make sure a distributed
+ * transaction is in progress.
+ */
+uint64
+CurrentDistributedTransactionNumber(void)
+{
+	Assert(MyBackendData != NULL);
+
+	return MyBackendData->transactionId.transactionNumber;
+}

src/backend/distributed/transaction/remote_transaction.c

@@ -20,6 +20,7 @@
 #include "distributed/metadata_cache.h"
 #include "distributed/remote_commands.h"
 #include "distributed/remote_transaction.h"
+#include "distributed/transaction_identifier.h"
 #include "distributed/transaction_management.h"
 #include "distributed/transaction_recovery.h"
 #include "distributed/worker_manager.h"
@@ -64,7 +65,7 @@ StartRemoteTransactionBegin(struct MultiConnection *connection)
 	 * and send both in one step. The reason is purely performance, we don't want
 	 * seperate roundtrips for these two statements.
 	 */
-	distributedTransactionId = GetCurrentDistributedTransctionId();
+	distributedTransactionId = GetCurrentDistributedTransactionId();
 	appendStringInfo(beginAndSetDistributedTransactionId,
 					 "SELECT assign_distributed_transaction_id(%d, %ld, '%s')",
 					 distributedTransactionId->initiatorNodeIdentifier,
@@ -856,11 +857,27 @@ CheckTransactionHealth(void)
 
 
 /*
- * Assign2PCIdentifier compute the 2PC transaction name to use for a
+ * Assign2PCIdentifier computes the 2PC transaction name to use for a
- * transaction.
+ * transaction. Every prepared transaction should get a new name, i.e. this
+ * function will need to be called again.
  *
- * Every 2PC transaction should get a new name, i.e. this function will need
+ * The format of the name is:
- * to be called again.
+ *
+ * citus_<source group>_<pid>_<distributed transaction number>_<connection number>
+ *
+ * (at most 5+1+10+1+10+20+1+10 = 58 characters, while limit is 64)
+ *
+ * The source group is used to distinguish 2PCs started by different
+ * coordinators. A coordinator will only attempt to recover its own 2PCs.
+ *
+ * The pid is used to distinguish different processes on the coordinator, mainly
+ * to provide some entropy across restarts.
+ *
+ * The distributed transaction number is used to distinguish different
+ * transactions originating from the same node (since restart).
+ *
+ * The connection number is used to distinguish connections made to a node
+ * within the same transaction.
  *
  * NB: we rely on the fact that we don't need to do full escaping on the names
  * generated here.
@@ -868,10 +885,16 @@ CheckTransactionHealth(void)
 static void
 Assign2PCIdentifier(MultiConnection *connection)
 {
-	static uint64 sequence = 0;
+	/* local sequence number used to distinguish different connections */
+	static uint32 connectionNumber = 0;
+
+	/* transaction identifier that is unique across processes */
+	uint64 transactionNumber = CurrentDistributedTransactionNumber();
+
+	/* print all numbers as unsigned to guarantee no minus symbols appear in the name */
 	snprintf(connection->remoteTransaction.preparedName, NAMEDATALEN,
-			 "citus_%d_%d_"UINT64_FORMAT, GetLocalGroupId(),
+			 "citus_%u_%u_"UINT64_FORMAT "_%u", GetLocalGroupId(), MyProcPid,
-			 MyProcPid, sequence++);
+			 transactionNumber, connectionNumber++);
 }
 
 

src/include/distributed/transaction_identifier.h

@@ -34,6 +34,7 @@ typedef struct DistributedTransactionId
 } DistributedTransactionId;
 
 
-extern DistributedTransactionId * GetCurrentDistributedTransctionId(void);
+extern DistributedTransactionId * GetCurrentDistributedTransactionId(void);
+extern uint64 CurrentDistributedTransactionNumber(void);
 
 #endif /* TRANSACTION_IDENTIFIER_H */