From 1e07a94435237462f63685c327d13e023549a10c Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 16 Jun 2016 14:49:00 -0700 Subject: [PATCH 1/2] Use cached comparator in ShardIntervalsOverlap(). By far the most expensive part of ShardIntervalsOverlap() is computing the function to use to determine overlap. Luckily we already have that computed and cached. The plan time for the query in issue #607 goes from 8764 ms to 2889 ms with this change. --- src/backend/distributed/planner/multi_physical_planner.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index f714aa3c3..3b800f09f 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -3601,17 +3601,16 @@ FragmentInterval(RangeTableFragment *fragment) bool ShardIntervalsOverlap(ShardInterval *firstInterval, ShardInterval *secondInterval) { - Oid typeId = InvalidOid; - FmgrInfo *comparisonFunction = NULL; bool nonOverlap = false; + DistTableCacheEntry *intervalRelation = + DistributedTableCacheEntry(firstInterval->relationId); + FmgrInfo *comparisonFunction = intervalRelation->shardIntervalCompareFunction; Datum firstMin = 0; Datum firstMax = 0; Datum secondMin = 0; Datum secondMax = 0; - typeId = firstInterval->valueTypeId; - comparisonFunction = GetFunctionInfo(typeId, BTREE_AM_OID, BTORDER_PROC); firstMin = firstInterval->minValue; firstMax = firstInterval->maxValue; From acb36b45052966cf2a74c20ebb18c1e666e9c3ce Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 16 Jun 2016 15:10:25 -0700 Subject: [PATCH 2/2] 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; } }