From 18a6e478af80b3b15a4905f8fc3c4e0b4046c4c7 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 20 Jul 2017 11:18:42 +0200 Subject: [PATCH 1/2] Fix typo in GetCurrentDistributedTransctionId --- src/backend/distributed/transaction/backend_data.c | 6 +++--- src/backend/distributed/transaction/remote_transaction.c | 2 +- src/include/distributed/transaction_identifier.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/backend/distributed/transaction/backend_data.c b/src/backend/distributed/transaction/backend_data.c index 8732b1a4f..44445b610 100644 --- a/src/backend/distributed/transaction/backend_data.c +++ b/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)); diff --git a/src/backend/distributed/transaction/remote_transaction.c b/src/backend/distributed/transaction/remote_transaction.c index d215948ef..eeb662256 100644 --- a/src/backend/distributed/transaction/remote_transaction.c +++ b/src/backend/distributed/transaction/remote_transaction.c @@ -64,7 +64,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, diff --git a/src/include/distributed/transaction_identifier.h b/src/include/distributed/transaction_identifier.h index 9e1de3f9d..64d333190 100644 --- a/src/include/distributed/transaction_identifier.h +++ b/src/include/distributed/transaction_identifier.h @@ -34,6 +34,6 @@ typedef struct DistributedTransactionId } DistributedTransactionId; -extern DistributedTransactionId * GetCurrentDistributedTransctionId(void); +extern DistributedTransactionId * GetCurrentDistributedTransactionId(void); #endif /* TRANSACTION_IDENTIFIER_H */ From 601b17d544a400c43116efe2d2ef2545ed0cfa50 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 20 Jul 2017 11:47:20 +0200 Subject: [PATCH 2/2] Use distributed transaction number in 2PC identifiers --- .../distributed/transaction/backend_data.c | 14 +++++++ .../transaction/remote_transaction.c | 37 +++++++++++++++---- .../distributed/transaction_identifier.h | 1 + 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/backend/distributed/transaction/backend_data.c b/src/backend/distributed/transaction/backend_data.c index 44445b610..e0d7ebc66 100644 --- a/src/backend/distributed/transaction/backend_data.c +++ b/src/backend/distributed/transaction/backend_data.c @@ -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; +} diff --git a/src/backend/distributed/transaction/remote_transaction.c b/src/backend/distributed/transaction/remote_transaction.c index eeb662256..b1960e4d7 100644 --- a/src/backend/distributed/transaction/remote_transaction.c +++ b/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" @@ -856,11 +857,27 @@ CheckTransactionHealth(void) /* - * Assign2PCIdentifier compute the 2PC transaction name to use for a - * transaction. + * Assign2PCIdentifier computes the 2PC transaction name to use for a + * 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 - * to be called again. + * The format of the name is: + * + * citus____ + * + * (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(), - MyProcPid, sequence++); + "citus_%u_%u_"UINT64_FORMAT "_%u", GetLocalGroupId(), MyProcPid, + transactionNumber, connectionNumber++); } diff --git a/src/include/distributed/transaction_identifier.h b/src/include/distributed/transaction_identifier.h index 64d333190..21c6d530e 100644 --- a/src/include/distributed/transaction_identifier.h +++ b/src/include/distributed/transaction_identifier.h @@ -35,5 +35,6 @@ typedef struct DistributedTransactionId extern DistributedTransactionId * GetCurrentDistributedTransactionId(void); +extern uint64 CurrentDistributedTransactionNumber(void); #endif /* TRANSACTION_IDENTIFIER_H */