When looking up the distributed XID for a local XID, go straight to the SLRU.

We used to scan the list of LocalDistribXactData objects, in shared memory,
to find the distributed XID corresponding a local XID, during visibility
checks. That turns out to be unnecessary: at the point when we scanned the
list, the distributed log SLRU has already been updated, so we might as well
check that directly.

Thanks to @ashwinstar for pointing this out.

src/backend/access/transam/xact.c

@@ -1309,16 +1309,19 @@ RecordTransactionCommit(void)
 		 */
 		if (markXidCommitted)
 		{
+			/*
+			 * Mark the distributed transaction committed. Note that this
+			 * is done *before* updating the clog. As soon as an XID is
+			 * marked as comitted in the clog, other backends might try
+			 * to look it up in the DistributedLog.
+			 */
+			/* UNDONE: What are the locking issues here? */
 			if (isDtxPrepared)
-			{
-				/* Mark the distributed transaction committed. */
-				/* UNDONE: What are the locking issues here? */
 				DistributedLog_SetCommitted(
 										xid,
 										getDtxStartTime(),
 										getDistributedTransactionId(),
 										/* isRedo */ false);
-			}
 
 			TransactionIdCommit(xid);
 			/* to avoid race conditions, the parent must commit first */

src/backend/cdb/cdbdistributedsnapshot.c

@@ -16,10 +16,13 @@
 #include "utils/tqual.h"
 
 /*
- * DistributedSnapshotWithLocalXids_CommittedTest
+ * DistributedSnapshotWithLocalMapping_CommittedTest
  *		Is the given XID still-in-progress according to the
  *      distributed snapshot?  Or, is the transaction strictly local
  *      and needs to be tested with the local snapshot?
+ *
+ * The caller should've checked that the XID is committed (in clog),
+ * otherwise the result of this function is undefined.
  */
 DistributedSnapshotCommitted 
 DistributedSnapshotWithLocalMapping_CommittedTest(
@@ -29,7 +32,6 @@ DistributedSnapshotWithLocalMapping_CommittedTest(
 {
 	DistributedSnapshotHeader *header = &dslm->header;
 	DistributedSnapshotMapEntry *inProgressEntryArray = dslm->inProgressEntryArray;
-	
 	int32							count;
 	uint32							i;
 	bool							found;
@@ -51,10 +53,9 @@ DistributedSnapshotWithLocalMapping_CommittedTest(
 	/*
 	 * Is this local xid in a process-local cache we maintain?
 	 */
-	found = LocalDistribXactCache_CommittedFind(
-											localXid, 
-											dslm->header.distribTransactionTimeStamp,
-											&distribXid);
+	found = LocalDistribXactCache_CommittedFind(localXid,
+												dslm->header.distribTransactionTimeStamp,
+												&distribXid);
 
 	if (found)
 	{
@@ -70,54 +71,40 @@ DistributedSnapshotWithLocalMapping_CommittedTest(
 	}
 	else
 	{
+		DistributedTransactionTimeStamp checkDistribTimeStamp;
+
 		/*
-		 * Small window -- but check if our LocalDistribXact element says
-		 * we are preparing this transaction.  If so, the CLOG knows the
-		 * commit and had the visibility routine call us before we've finished
-		 * updating our data structures.
+		 * Ok, now we must consult the distributed log.
 		 */
-		if (!LocalDistribXact_LocalXidKnown(
-										localXid,
-										dslm->header.distribTransactionTimeStamp,
-										&distribXid))
+		if (DistributedLog_CommittedCheck(localXid,
+										  &checkDistribTimeStamp,
+										  &distribXid))
 		{
-			DistributedTransactionTimeStamp checkDistribTimeStamp;
-
 			/*
-			 * Ok, now we must consult the distributed log.
+			 * We found it in the distributed log.
 			 */
-			if (DistributedLog_CommittedCheck(
-									localXid,
-									&checkDistribTimeStamp,
-									&distribXid))
-			{
-				/*
-				 * We found it in the distributed log.
-				 */
-				Assert(checkDistribTimeStamp != 0);
-				Assert(distribXid != InvalidDistributedTransactionId);
-
-				/*
-				 * Committed distributed transactions from other DTM starts are
-				 * weeded out.
-				 */
-				if (checkDistribTimeStamp != header->distribTransactionTimeStamp)
-					return DISTRIBUTEDSNAPSHOT_COMMITTED_IGNORE;
-			}
-			else
-			{
-				/*
-				 * Since the local xid is committed (as determined by the
-				 * visibility routine) and all of our data structures do not
-				 * know of the transaction, it must be local-only.
-				 */
-				LocalDistribXactCache_AddCommitted(
-												localXid, 
-												dslm->header.distribTransactionTimeStamp,
-												/* distribXid */ InvalidDistributedTransactionId);
+			Assert(checkDistribTimeStamp != 0);
+			Assert(distribXid != InvalidDistributedTransactionId);
 
+			/*
+			 * Committed distributed transactions from other DTM starts are
+			 * weeded out.
+			 */
+			if (checkDistribTimeStamp != header->distribTransactionTimeStamp)
 				return DISTRIBUTEDSNAPSHOT_COMMITTED_IGNORE;
-			}
+		}
+		else
+		{
+			/*
+			 * Since the local xid is committed (as determined by the
+			 * visibility routine) and distributedlog doesn't know of the
+			 * transaction, it must be local-only.
+			 */
+			LocalDistribXactCache_AddCommitted(localXid,
+											   dslm->header.distribTransactionTimeStamp,
+											   /* distribXid */ InvalidDistributedTransactionId);
+
+			return DISTRIBUTEDSNAPSHOT_COMMITTED_IGNORE;
 		}
 	}
 	

src/backend/cdb/cdblocaldistribxact.c

@@ -328,37 +328,6 @@ LocalDistribXact_GetMaxDistributedXid(void)
 
 }
 
-static LocalDistribXact
-LocalDistribXact_FindByLocalXid(
-	DistributedTransactionId 	searchLocalXid)
-{
-	LocalDistribXact 	ele;
-
-	Assert(LocalDistribXactShared != NULL);
-
-	ele = (LocalDistribXact)
-					SharedDoublyLinkedHead_First(
-									&LocalDistribXactShared->sortedLocalBase,
-									&LocalDistribXactShared->sortedLocalList);
-	while (ele != NULL)
-	{
-		if (ele->localXid == searchLocalXid)
-		{
-			return ele;
-		}
-		else if (ele->localXid > searchLocalXid)
-			break;
-
-		ele = (LocalDistribXact)
-					SharedDoubleLinks_Next(
-									&LocalDistribXactShared->sortedLocalBase,
-									&LocalDistribXactShared->sortedLocalList,
-									ele);
-	}
-
-	return NULL;
-}
-
 static void
 LocalDistribXact_SetLocalPgProc(
 	LocalDistribXact	ele)
@@ -710,32 +679,6 @@ LocalDistribXact_DisplayString(
 	return LocalDistribDisplayBuffer;
 }
 
-bool
-LocalDistribXact_LocalXidKnown(
-	TransactionId						localXid,
-	DistributedTransactionTimeStamp		distribTimeStamp,
-	DistributedTransactionId 			*distribXid)
-{
-	LocalDistribXact 	ele;
-
-	LWLockAcquire(ProcArrayLock, LW_SHARED);
-
-	ele = LocalDistribXact_FindByLocalXid(localXid);
-	if (ele == NULL || ele->distribTimeStamp != distribTimeStamp)
-	{
-		LWLockRelease(ProcArrayLock);
-
-		*distribXid = InvalidDistributedTransactionId;
-		return false;
-	}
-
-	*distribXid = ele->distribXid;
-
-	LWLockRelease(ProcArrayLock);
-
-	return true;
-}
-
 // *****************************************************************************
 
 static int

src/include/cdb/cdblocaldistribxact.h

@@ -83,11 +83,6 @@ extern void LocalDistribXact_GetDistributedXid(
 extern char* LocalDistribXact_DisplayString(
 	LocalDistribXactRef		*localDistribXactRef);
 
-extern bool LocalDistribXact_LocalXidKnown(
-	TransactionId						localXid,
-	DistributedTransactionTimeStamp		distribTimeStamp,
-	DistributedTransactionId 			*distribXid);
-
 extern bool LocalDistribXactCache_CommittedFind(
 	TransactionId						localXid,
 	DistributedTransactionTimeStamp		distribTransactionTimeStamp,