From 0ea4e52df512acd678ed7ad0cb128638221a3a5a Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 7 Mar 2019 11:42:03 +0100 Subject: [PATCH] Add nodeId to shardPlacements and use it for shard placement comparisons Before this commit, shardPlacements were identified with shardId, nodeName and nodeport. Instead of using nodeName and nodePort, we now use nodeId since it apparently has performance benefits in several places in the code. --- src/backend/distributed/commands/multi_copy.c | 7 +++++++ .../connection/placement_connection.c | 12 ++++-------- .../planner/multi_physical_planner.c | 4 ++-- .../distributed/planner/multi_router_planner.c | 1 + src/backend/distributed/utils/citus_copyfuncs.c | 1 + src/backend/distributed/utils/citus_outfuncs.c | 1 + src/backend/distributed/utils/citus_readfuncs.c | 1 + .../distributed/utils/colocation_utils.c | 17 ++--------------- src/backend/distributed/utils/metadata_cache.c | 1 + .../distributed/master_metadata_utility.h | 1 + 10 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index f22878f6a..a96d87529 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -1051,6 +1051,13 @@ RemoteFinalizedShardPlacementList(uint64 shardId) shardPlacement->nodeName = nodeName; shardPlacement->nodePort = nodePort; + /* + * We cannot know the nodeId, but it is not necessary at this point either. + * This is only used to to look up the connection for a group of co-located + * placements, but append-distributed tables are never co-located. + */ + shardPlacement->nodeId = -1; + finalizedPlacementList = lappend(finalizedPlacementList, shardPlacement); } } diff --git a/src/backend/distributed/connection/placement_connection.c b/src/backend/distributed/connection/placement_connection.c index 15e7b7551..20303e58b 100644 --- a/src/backend/distributed/connection/placement_connection.c +++ b/src/backend/distributed/connection/placement_connection.c @@ -133,8 +133,7 @@ static HTAB *ConnectionPlacementHash; typedef struct ColocatedPlacementsHashKey { /* to identify host - database can't differ */ - char nodeName[MAX_NODE_LENGTH]; - uint32 nodePort; + int nodeId; /* colocation group, or invalid */ uint32 colocationGroupId; @@ -689,8 +688,7 @@ FindOrCreatePlacementEntry(ShardPlacement *placement) ColocatedPlacementsHashKey key; ColocatedPlacementsHashEntry *colocatedEntry = NULL; - strlcpy(key.nodeName, placement->nodeName, MAX_NODE_LENGTH); - key.nodePort = placement->nodePort; + key.nodeId = placement->nodeId; key.colocationGroupId = placement->colocationGroupId; key.representativeValue = placement->representativeValue; @@ -1149,8 +1147,7 @@ ColocatedPlacementsHashHash(const void *key, Size keysize) ColocatedPlacementsHashKey *entry = (ColocatedPlacementsHashKey *) key; uint32 hash = 0; - hash = string_hash(entry->nodeName, NAMEDATALEN); - hash = hash_combine(hash, hash_uint32(entry->nodePort)); + hash = hash_uint32(entry->nodeId); hash = hash_combine(hash, hash_uint32(entry->colocationGroupId)); hash = hash_combine(hash, hash_uint32(entry->representativeValue)); @@ -1164,8 +1161,7 @@ ColocatedPlacementsHashCompare(const void *a, const void *b, Size keysize) ColocatedPlacementsHashKey *ca = (ColocatedPlacementsHashKey *) a; ColocatedPlacementsHashKey *cb = (ColocatedPlacementsHashKey *) b; - if (strncmp(ca->nodeName, cb->nodeName, MAX_NODE_LENGTH) != 0 || - ca->nodePort != cb->nodePort || + if (ca->nodeId != cb->nodeId || ca->colocationGroupId != cb->colocationGroupId || ca->representativeValue != cb->representativeValue) { diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index 8e359239f..a12572ad5 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -2552,8 +2552,7 @@ CoPlacedShardIntervals(ShardInterval *firstInterval, ShardInterval *secondInterv ShardPlacement *secondShardPlacement = (ShardPlacement *) lfirst( secondShardPlacementCell); - if (strcmp(firstShardPlacement->nodeName, secondShardPlacement->nodeName) != 0 || - firstShardPlacement->nodePort != secondShardPlacement->nodePort) + if (firstShardPlacement->nodeId != secondShardPlacement->nodeId) { return false; } @@ -5375,6 +5374,7 @@ AssignDualHashTaskList(List *taskList) ShardPlacement *taskPlacement = CitusMakeNode(ShardPlacement); taskPlacement->nodeName = pstrdup(workerNode->workerName); taskPlacement->nodePort = workerNode->workerPort; + taskPlacement->nodeId = workerNode->nodeId; taskPlacementList = lappend(taskPlacementList, taskPlacement); } diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index 8694e44ed..854c40309 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -2027,6 +2027,7 @@ PlanRouterQuery(Query *originalQuery, (ShardPlacement *) CitusMakeNode(ShardPlacement); dummyPlacement->nodeName = workerNode->workerName; dummyPlacement->nodePort = workerNode->workerPort; + dummyPlacement->nodeId = workerNode->nodeId; dummyPlacement->groupId = workerNode->groupId; workerList = lappend(workerList, dummyPlacement); diff --git a/src/backend/distributed/utils/citus_copyfuncs.c b/src/backend/distributed/utils/citus_copyfuncs.c index 757317851..6d302e441 100644 --- a/src/backend/distributed/utils/citus_copyfuncs.c +++ b/src/backend/distributed/utils/citus_copyfuncs.c @@ -200,6 +200,7 @@ CopyNodeShardPlacement(COPYFUNC_ARGS) COPY_SCALAR_FIELD(groupId); COPY_STRING_FIELD(nodeName); COPY_SCALAR_FIELD(nodePort); + COPY_SCALAR_FIELD(nodeId); COPY_SCALAR_FIELD(partitionMethod); COPY_SCALAR_FIELD(colocationGroupId); COPY_SCALAR_FIELD(representativeValue); diff --git a/src/backend/distributed/utils/citus_outfuncs.c b/src/backend/distributed/utils/citus_outfuncs.c index ca8f77d67..94caafa0e 100644 --- a/src/backend/distributed/utils/citus_outfuncs.c +++ b/src/backend/distributed/utils/citus_outfuncs.c @@ -405,6 +405,7 @@ OutShardPlacement(OUTFUNC_ARGS) WRITE_INT_FIELD(groupId); WRITE_STRING_FIELD(nodeName); WRITE_UINT_FIELD(nodePort); + WRITE_INT_FIELD(nodeId); /* so we can deal with 0 */ WRITE_INT_FIELD(partitionMethod); WRITE_UINT_FIELD(colocationGroupId); diff --git a/src/backend/distributed/utils/citus_readfuncs.c b/src/backend/distributed/utils/citus_readfuncs.c index cbd573142..93df55fab 100644 --- a/src/backend/distributed/utils/citus_readfuncs.c +++ b/src/backend/distributed/utils/citus_readfuncs.c @@ -315,6 +315,7 @@ ReadShardPlacement(READFUNC_ARGS) READ_INT_FIELD(groupId); READ_STRING_FIELD(nodeName); READ_UINT_FIELD(nodePort); + READ_INT_FIELD(nodeId); /* so we can deal with 0 */ READ_INT_FIELD(partitionMethod); READ_UINT_FIELD(colocationGroupId); diff --git a/src/backend/distributed/utils/colocation_utils.c b/src/backend/distributed/utils/colocation_utils.c index ff273a99c..89e3cebb7 100644 --- a/src/backend/distributed/utils/colocation_utils.c +++ b/src/backend/distributed/utils/colocation_utils.c @@ -420,25 +420,12 @@ CompareShardPlacementsByNode(const void *leftElement, const void *rightElement) const ShardPlacement *leftPlacement = *((const ShardPlacement **) leftElement); const ShardPlacement *rightPlacement = *((const ShardPlacement **) rightElement); - char *leftNodeName = leftPlacement->nodeName; - char *rightNodeName = rightPlacement->nodeName; - - uint32 leftNodePort = leftPlacement->nodePort; - uint32 rightNodePort = rightPlacement->nodePort; - - /* first compare node names */ - int nodeNameCompare = strncmp(leftNodeName, rightNodeName, WORKER_LENGTH); - if (nodeNameCompare != 0) - { - return nodeNameCompare; - } - /* if node names are same, check node ports */ - if (leftNodePort < rightNodePort) + if (leftPlacement->nodeId < rightPlacement->nodeId) { return -1; } - else if (leftNodePort > rightNodePort) + else if (leftPlacement->nodeId > rightPlacement->nodeId) { return 1; } diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index f1e0e1f9a..1c397daab 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -526,6 +526,7 @@ ResolveGroupShardPlacement(GroupShardPlacement *groupShardPlacement, shardPlacement->nodeName = pstrdup(workerNode->workerName); shardPlacement->nodePort = workerNode->workerPort; + shardPlacement->nodeId = workerNode->nodeId; /* fill in remaining fields */ Assert(tableEntry->partitionMethod != 0); diff --git a/src/include/distributed/master_metadata_utility.h b/src/include/distributed/master_metadata_utility.h index 3fba2b449..093b0697b 100644 --- a/src/include/distributed/master_metadata_utility.h +++ b/src/include/distributed/master_metadata_utility.h @@ -99,6 +99,7 @@ typedef struct ShardPlacement /* the rest of the fields aren't from pg_dist_placement */ char *nodeName; uint32 nodePort; + int nodeId; char partitionMethod; uint32 colocationGroupId; uint32 representativeValue;