From 2e8e8d377eace314f86dcc830df5d32b18c4d422 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 16 Jun 2016 15:10:25 -0700 Subject: [PATCH] Store ShardInterval instead of shardId in RangeTableFragments. For CITUS_RTE_RELATION type fragments, reloading shardIntervals from the database is rather expensive. So store a pointer to the full shard interval, instead of just the shard id. There's no new memory lifetime hazards here, because we already passed a pointer to the shardInterval's ->shardId field around. The plan time for the query in issue #607 goes from 2889 ms to 106 ms. with this change. --- .../planner/multi_physical_planner.c | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index 3b800f09f..832e95d54 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -2008,7 +2008,7 @@ SubquerySqlTaskList(Job *job) ShardInterval *shardInterval = (ShardInterval *) lfirst(shardIntervalCell); RangeTableFragment *shardFragment = palloc0(fragmentSize); - shardFragment->fragmentReference = &(shardInterval->shardId); + shardFragment->fragmentReference = shardInterval; shardFragment->fragmentType = CITUS_RTE_RELATION; shardFragment->rangeTableId = tableId; @@ -2449,7 +2449,7 @@ RangeTableFragmentsList(List *rangeTableList, List *whereClauseList, (ShardInterval *) lfirst(shardIntervalCell); RangeTableFragment *shardFragment = palloc0(fragmentSize); - shardFragment->fragmentReference = &shardInterval->shardId; + shardFragment->fragmentReference = shardInterval; shardFragment->fragmentType = CITUS_RTE_RELATION; shardFragment->rangeTableId = tableId; @@ -3582,14 +3582,16 @@ FragmentInterval(RangeTableFragment *fragment) ShardInterval *fragmentInterval = NULL; if (fragment->fragmentType == CITUS_RTE_RELATION) { - uint64 *shardId = (uint64 *) fragment->fragmentReference; - - fragmentInterval = LoadShardInterval(*shardId); + Assert(CitusIsA(fragment->fragmentReference, ShardInterval)); + fragmentInterval = (ShardInterval *) fragment->fragmentReference; } else if (fragment->fragmentType == CITUS_RTE_REMOTE_QUERY) { - Task *mergeTask = (Task *) fragment->fragmentReference; + Task *mergeTask = NULL; + Assert(CitusIsA(fragment->fragmentReference, Task)); + + mergeTask = (Task *) fragment->fragmentReference; fragmentInterval = mergeTask->shardInterval; } @@ -3687,22 +3689,24 @@ UniqueFragmentList(List *fragmentList) foreach(fragmentCell, fragmentList) { - uint64 *shardId = NULL; + ShardInterval *shardInterval = NULL; bool shardIdAlreadyAdded = false; ListCell *uniqueFragmentCell = NULL; RangeTableFragment *fragment = (RangeTableFragment *) lfirst(fragmentCell); Assert(fragment->fragmentType == CITUS_RTE_RELATION); - shardId = fragment->fragmentReference; + Assert(CitusIsA(fragment->fragmentReference, ShardInterval)); + shardInterval = (ShardInterval *) fragment->fragmentReference; foreach(uniqueFragmentCell, uniqueFragmentList) { RangeTableFragment *uniqueFragment = (RangeTableFragment *) lfirst(uniqueFragmentCell); - uint64 *uniqueShardId = uniqueFragment->fragmentReference; + ShardInterval *uniqueShardInterval = + (ShardInterval *) uniqueFragment->fragmentReference; - if (*shardId == *uniqueShardId) + if (shardInterval->shardId == uniqueShardInterval->shardId) { shardIdAlreadyAdded = true; break; @@ -3734,12 +3738,13 @@ DataFetchTaskList(uint64 jobId, uint32 taskIdIndex, List *fragmentList) RangeTableFragment *fragment = (RangeTableFragment *) lfirst(fragmentCell); if (fragment->fragmentType == CITUS_RTE_RELATION) { - uint64 *shardId = fragment->fragmentReference; - StringInfo shardFetchQueryString = ShardFetchQueryString(*shardId); + ShardInterval *shardInterval = fragment->fragmentReference; + uint64 shardId = shardInterval->shardId; + StringInfo shardFetchQueryString = ShardFetchQueryString(shardId); Task *shardFetchTask = CreateBasicTask(jobId, taskIdIndex, SHARD_FETCH_TASK, shardFetchQueryString->data); - shardFetchTask->shardId = (*shardId); + shardFetchTask->shardId = shardId; dataFetchTaskList = lappend(dataFetchTaskList, shardFetchTask); taskIdIndex++; @@ -3969,7 +3974,8 @@ FragmentAlias(RangeTblEntry *rangeTableEntry, RangeTableFragment *fragment) if (fragmentType == CITUS_RTE_RELATION) { char *shardAliasName = NULL; - uint64 *shardId = fragment->fragmentReference; + ShardInterval *shardInterval = (ShardInterval *) fragment->fragmentReference; + uint64 shardId = shardInterval->shardId; Oid relationId = rangeTableEntry->relid; char *relationName = get_rel_name(relationId); @@ -3991,7 +3997,7 @@ FragmentAlias(RangeTblEntry *rangeTableEntry, RangeTableFragment *fragment) * If user specified a shard name in pg_dist_shard, use that name in alias. * Otherwise, set shard name in alias to _. */ - shardAliasName = LoadShardAlias(relationId, *shardId); + shardAliasName = LoadShardAlias(relationId, shardId); if (shardAliasName != NULL) { fragmentName = shardAliasName; @@ -3999,7 +4005,7 @@ FragmentAlias(RangeTblEntry *rangeTableEntry, RangeTableFragment *fragment) else { char *shardName = pstrdup(relationName); - AppendShardIdToName(&shardName, *shardId); + AppendShardIdToName(&shardName, shardId); fragmentName = shardName; } @@ -4057,11 +4063,13 @@ AnchorShardId(List *fragmentList, uint32 anchorRangeTableId) RangeTableFragment *fragment = (RangeTableFragment *) lfirst(fragmentCell); if (fragment->rangeTableId == anchorRangeTableId) { - uint64 *shardIdPointer = NULL; - Assert(fragment->fragmentType == CITUS_RTE_RELATION); + ShardInterval *shardInterval = NULL; - shardIdPointer = (uint64 *) fragment->fragmentReference; - anchorShardId = (*shardIdPointer); + Assert(fragment->fragmentType == CITUS_RTE_RELATION); + Assert(CitusIsA(fragment->fragmentReference, ShardInterval)); + + shardInterval = (ShardInterval *) fragment->fragmentReference; + anchorShardId = shardInterval->shardId; break; } }