From 7d0f7835e74d784a3f8900ef9c0a7920e777d726 Mon Sep 17 00:00:00 2001
From: Onder Kalaci <onder@citusdata.com>
Date: Mon, 25 Jun 2018 10:24:21 +0300
Subject: [PATCH] Improve relation accesses association to do less job

---
 .../transaction/relation_access_tracking.c    | 41 ++++++++++---------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/backend/distributed/transaction/relation_access_tracking.c b/src/backend/distributed/transaction/relation_access_tracking.c
index 59a759293..3c0d495a5 100644
--- a/src/backend/distributed/transaction/relation_access_tracking.c
+++ b/src/backend/distributed/transaction/relation_access_tracking.c
@@ -66,6 +66,7 @@ typedef struct RelationAccessHashEntry
 static HTAB *RelationAccessHash;
 
 
+static void RecordRelationAccess(Oid relationId, ShardPlacementAccessType accessType);
 static void RecordPlacementAccessToCache(Oid relationId,
 										 ShardPlacementAccessType accessType);
 static RelationAccessMode GetRelationAccessMode(Oid relationId,
@@ -115,8 +116,8 @@ void
 AssociatePlacementAccessWithRelation(ShardPlacement *placement,
 									 ShardPlacementAccessType accessType)
 {
-	uint64 shardId = INVALID_SHARD_ID;
 	Oid relationId = InvalidOid;
+	uint64 shardId = INVALID_SHARD_ID;
 
 	if (!ShouldRecordRelationAccess())
 	{
@@ -126,6 +127,21 @@ AssociatePlacementAccessWithRelation(ShardPlacement *placement,
 	shardId = placement->shardId;
 	relationId = RelationIdForShard(shardId);
 
+	RecordRelationAccess(relationId, accessType);
+}
+
+
+/*
+ * RecordRelationAccess associates the access to the distributed relation. The
+ * function takes partitioned relations into account as well.
+ *
+ * We implemented this function to prevent accessing placement metadata during
+ * recursive calls of the function itself (e.g., avoid
+ * AssociatePlacementAccessWithRelation()).
+ */
+static void
+RecordRelationAccess(Oid relationId, ShardPlacementAccessType accessType)
+{
 	/*
 	 * If a relation is partitioned, record accesses to all of its partitions as well.
 	 * We prefer to use PartitionedTableNoLock() because at this point the necessary
@@ -134,16 +150,11 @@ AssociatePlacementAccessWithRelation(ShardPlacement *placement,
 	if (PartitionedTableNoLock(relationId))
 	{
 		List *partitionList = PartitionList(relationId);
-		ShardInterval *shardInterval = LoadShardInterval(shardId);
-		int shardIndex = shardInterval->shardIndex;
 		ListCell *partitionCell = NULL;
 
 		foreach(partitionCell, partitionList)
 		{
 			Oid partitionOid = lfirst_oid(partitionCell);
-			uint64 partitionShardId = INVALID_SHARD_ID;
-			List *partitionShardPlacementList = NIL;
-			ShardPlacement *partitionInitialShardPlacement = NULL;
 
 			/*
 			 * During create_distributed_table, the partitions may not
@@ -156,29 +167,19 @@ AssociatePlacementAccessWithRelation(ShardPlacement *placement,
 				continue;
 			}
 
-			partitionShardId =
-				ColocatedShardIdInRelation(partitionOid, shardIndex);
-			partitionShardPlacementList = ShardPlacementList(partitionShardId);
-			partitionInitialShardPlacement =
-				(ShardPlacement *) linitial(partitionShardPlacementList);
-
-			/*
-			 * Recursively record all relation accesses of its partitions. Note that
-			 * we prefer to recursively call AssociatePlacementAccessWithRelation()
-			 * to support multi-level partitioned tables.
-			 */
-			AssociatePlacementAccessWithRelation(partitionInitialShardPlacement,
-												 accessType);
+			/* recursively call the function to cover multi-level partitioned tables */
+			RecordRelationAccess(partitionOid, accessType);
 		}
 	}
 	else if (PartitionTableNoLock(relationId))
 	{
 		Oid parentOid = PartitionParentOid(relationId);
 
-		/* only record the parent */
+		/* record the parent */
 		RecordPlacementAccessToCache(parentOid, accessType);
 	}
 
+	/* always record the relation that is being considered */
 	RecordPlacementAccessToCache(relationId, accessType);
 }