From 42b5877fd9bc1ec66527277e4bdac510b63433f3 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Wed, 11 Aug 2021 17:39:53 +0200 Subject: [PATCH 1/9] prototype for forceOneShardAttributeValue --- arangod/Aql/QueryOptions.cpp | 42 ++++++++++++------------- arangod/Aql/QueryOptions.h | 4 +++ arangod/Aql/ShardLocking.cpp | 60 +++++++++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/arangod/Aql/QueryOptions.cpp b/arangod/Aql/QueryOptions.cpp index 9cfe215c231b..79146a4cf8c5 100644 --- a/arangod/Aql/QueryOptions.cpp +++ b/arangod/Aql/QueryOptions.cpp @@ -172,50 +172,43 @@ void QueryOptions::fromVelocyPack(VPackSlice slice) { traversalProfile = static_cast(value.getNumber()); } - value = slice.get("allPlans"); - if (value.isBool()) { + if (value = slice.get("allPlans"); value.isBool()) { allPlans = value.getBool(); } - value = slice.get("verbosePlans"); - if (value.isBool()) { + if (value = slice.get("verbosePlans"); value.isBool()) { verbosePlans = value.getBool(); } - value = slice.get("stream"); - if (value.isBool()) { + if (value = slice.get("stream"); value.isBool()) { stream = value.getBool(); } - value = slice.get("silent"); - if (value.isBool()) { + if (value = slice.get("silent"); value.isBool()) { silent = value.getBool(); } - value = slice.get("failOnWarning"); - if (value.isBool()) { + if (value = slice.get("failOnWarning"); value.isBool()) { failOnWarning = value.getBool(); } - value = slice.get("cache"); - if (value.isBool()) { + if (value = slice.get("cache"); value.isBool()) { cache = value.getBool(); } - value = slice.get("fullCount"); - if (value.isBool()) { + if (value = slice.get("fullCount"); value.isBool()) { fullCount = value.getBool(); } - value = slice.get("count"); - if (value.isBool()) { + if (value = slice.get("count"); value.isBool()) { count = value.getBool(); } - value = slice.get("verboseErrors"); - if (value.isBool()) { + if (value = slice.get("verboseErrors"); value.isBool()) { verboseErrors = value.getBool(); } - value = slice.get("explainRegisters"); - if (value.isBool()) { - explainRegisters = - value.getBool() ? ExplainRegisterPlan::Yes : ExplainRegisterPlan::No; + if (value = slice.get("explainRegisters"); value.isBool()) { + explainRegisters = value.getBool() ? ExplainRegisterPlan::Yes : ExplainRegisterPlan::No; } - + // note: skipAudit is intentionally not read here. // the end user cannot override this setting + + if (value = slice.get("forceOneShardAttributeValue"); value.isString()) { + forceOneShardAttributeValue = value.copyString(); + } VPackSlice optimizer = slice.get("optimizer"); if (optimizer.isObject()) { @@ -279,6 +272,9 @@ void QueryOptions::toVelocyPack(VPackBuilder& builder, bool disableOptimizerRule builder.add("fullCount", VPackValue(fullCount)); builder.add("count", VPackValue(count)); builder.add("verboseErrors", VPackValue(verboseErrors)); + if (!forceOneShardAttributeValue.empty()) { + builder.add("forceOneShardAttributeValue", VPackValue(forceOneShardAttributeValue)); + } // note: skipAudit is intentionally not serialized here. // the end user cannot override this setting anyway. diff --git a/arangod/Aql/QueryOptions.h b/arangod/Aql/QueryOptions.h index ce6791a9e6cf..bab5183cae88 100644 --- a/arangod/Aql/QueryOptions.h +++ b/arangod/Aql/QueryOptions.h @@ -92,6 +92,10 @@ struct QueryOptions { bool skipAudit; // skips audit logging - used only internally ExplainRegisterPlan explainRegisters; + /// @brief shard key attribute value used to push a query down + /// to a single server + std::string forceOneShardAttributeValue; + /// @brief optimizer rules to turn off/on manually std::vector optimizerRules; diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 81b83c98d147..8050a8239d54 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -44,6 +44,28 @@ std::unordered_set const ShardLocking::EmptyShardListUnordered{}; void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, bool pushToSingleServer) { TRI_ASSERT(baseNode != nullptr); + + std::string const& forceOneShardAttributeValue = _query.queryOptions().forceOneShardAttributeValue; + + auto addRestrictedShard = [&](aql::Collection const* col, + std::unordered_set& restrictedShards) { + TRI_ASSERT(!forceOneShardAttributeValue.empty()); + std::string shardId; + auto errorCode = TRI_ERROR_NO_ERROR; + if (col->isDisjoint()) { + // if disjoint smart edge collection, we must insert an + // artifical key with two colons, to pretend it is a real + // smart graph key + errorCode = col->getCollection()->getResponsibleShard(forceOneShardAttributeValue + ":test:" + forceOneShardAttributeValue, shardId); + } else { + errorCode = col->getCollection()->getResponsibleShard(forceOneShardAttributeValue, shardId); + } + if (errorCode != TRI_ERROR_NO_ERROR) { + THROW_ARANGO_EXCEPTION(errorCode); + } + restrictedShards.emplace(shardId); + }; + // If we have ever accessed the server lists, // we cannot insert Nodes anymore. // If this needs to be modified in the future, this could @@ -67,13 +89,21 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, }; // Add all Edge Collections to the Transactions, Traversals do never write for (auto const& col : graphNode->edgeColls()) { - updateLocking(col, AccessMode::Type::READ, snippetId, {}, isUsedAsSatellite(col)); + std::unordered_set restrictedShards; + if (!forceOneShardAttributeValue.empty()) { + addRestrictedShard(col, restrictedShards); + } + updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, isUsedAsSatellite(col)); } // Add all Vertex Collections to the Transactions, Traversals do never // write, the collections have been adjusted already for (auto const& col : graphNode->vertexColls()) { - updateLocking(col, AccessMode::Type::READ, snippetId, {}, isUsedAsSatellite(col)); + std::unordered_set restrictedShards; + if (!forceOneShardAttributeValue.empty()) { + addRestrictedShard(col, restrictedShards); + } + updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, isUsedAsSatellite(col)); } break; } @@ -85,13 +115,15 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, TRI_ERROR_INTERNAL, "unable to cast node to CollectionAccessingNode"); } - std::unordered_set restrictedShard; - if (colNode->isRestricted()) { - restrictedShard.emplace(colNode->restrictedShard()); + std::unordered_set restrictedShards; + if (!forceOneShardAttributeValue.empty()) { + addRestrictedShard(colNode->collection(), restrictedShards); + } else if (colNode->isRestricted()) { + restrictedShards.emplace(colNode->restrictedShard()); } auto* col = colNode->collection(); - updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShard, + updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, colNode->isUsedAsSatellite()); break; } @@ -102,7 +134,11 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, "unable to cast node to ViewNode"); } for (aql::Collection const& col : viewNode->collections()) { - updateLocking(&col, AccessMode::Type::READ, snippetId, {}, false); + std::unordered_set restrictedShards; + if (!forceOneShardAttributeValue.empty()) { + addRestrictedShard(&col, restrictedShards); + } + updateLocking(&col, AccessMode::Type::READ, snippetId, restrictedShards, false); } break; @@ -119,16 +155,18 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, } auto* col = modNode->collection(); - std::unordered_set restrictedShard; - if (modNode->isRestricted()) { - restrictedShard.emplace(modNode->restrictedShard()); + std::unordered_set restrictedShards; + if (!forceOneShardAttributeValue.empty()) { + addRestrictedShard(modNode->collection(), restrictedShards); + } else if (modNode->isRestricted()) { + restrictedShards.emplace(modNode->restrictedShard()); } // Not supported yet TRI_ASSERT(!modNode->isUsedAsSatellite()); updateLocking(col, modNode->getOptions().exclusive ? AccessMode::Type::EXCLUSIVE : AccessMode::Type::WRITE, - snippetId, restrictedShard, modNode->isUsedAsSatellite()); + snippetId, restrictedShards, modNode->isUsedAsSatellite()); break; } default: From b2a119f9657c72333ad40c20cf9eecc926463a4c Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 24 Aug 2021 10:44:02 +0200 Subject: [PATCH 2/9] don't crash, but fail gracefully --- .../Aql/EngineInfoContainerDBServerServerBased.cpp | 14 ++++++++++---- arangod/Aql/GraphNode.cpp | 7 +++++++ arangod/Aql/ShardLocking.cpp | 4 +++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp index 054c7f6fc76e..ef297b3c307c 100644 --- a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp +++ b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp @@ -87,6 +87,7 @@ EngineInfoContainerDBServerServerBased::TraverserEngineShardLists::TraverserEngi auto const& restrictToShards = query.queryOptions().restrictToShards; // Extract the local shards for edge collections. for (auto const& col : edges) { + TRI_ASSERT(col != nullptr); #ifdef USE_ENTERPRISE if (query.trxForOptimization().isInaccessibleCollection(col->id())) { _inaccessible.insert(col->name()); @@ -103,6 +104,7 @@ EngineInfoContainerDBServerServerBased::TraverserEngineShardLists::TraverserEngi // It might in fact be empty, if we only have edge collections in a graph. // Or if we guarantee to never read vertex data. for (auto const& col : vertices) { + TRI_ASSERT(col != nullptr); #ifdef USE_ENTERPRISE if (query.trxForOptimization().isInaccessibleCollection(col->id())) { _inaccessible.insert(col->name()); @@ -120,7 +122,11 @@ std::vector EngineInfoContainerDBServerServerBased::TraverserEngineShar std::vector localShards; for (auto const& shard : *shardIds) { auto const& it = shardMapping.find(shard); - TRI_ASSERT(it != shardMapping.end()); + if (it == shardMapping.end()) { + THROW_ARANGO_EXCEPTION_MESSAGE( + TRI_ERROR_INTERNAL, + "no entry for shard '" + shard + "' in shard mapping table (" + std::to_string(shardMapping.size()) + " entries)"); + } if (it->second == server) { localShards.emplace_back(shard); _hasShard = true; @@ -853,7 +859,7 @@ void EngineInfoContainerDBServerServerBased::addOptionsPart(arangodb::velocypack #endif } -// Insert the Variables information into the message to be send to DBServers +// Insert the Variables information into the message to be sent to DBServers void EngineInfoContainerDBServerServerBased::addVariablesPart(arangodb::velocypack::Builder& builder) const { TRI_ASSERT(builder.isOpenObject()); builder.add(VPackValue("variables")); @@ -861,7 +867,7 @@ void EngineInfoContainerDBServerServerBased::addVariablesPart(arangodb::velocypa _query.ast()->variables()->toVelocyPack(builder); } -// Insert the Snippets information into the message to be send to DBServers +// Insert the Snippets information into the message to be sent to DBServers void EngineInfoContainerDBServerServerBased::addSnippetPart( std::unordered_map const& nodesById, arangodb::velocypack::Builder& builder, ShardLocking& shardLocking, @@ -875,7 +881,7 @@ void EngineInfoContainerDBServerServerBased::addSnippetPart( builder.close(); // snippets } -// Insert the TraversalEngine information into the message to be send to DBServers +// Insert the TraversalEngine information into the message to be sent to DBServers std::vector EngineInfoContainerDBServerServerBased::addTraversalEnginesPart( arangodb::velocypack::Builder& infoBuilder, std::unordered_map const& shardMapping, ServerID const& server) const { diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index 7d5d3179e0ff..f9c0d6f200ee 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -466,12 +466,14 @@ void GraphNode::setGraphInfoAndCopyColls(std::vector const& edgeCol std::vector const& vertexColls) { _graphInfo.openArray(); for (auto& it : edgeColls) { + TRI_ASSERT(it != nullptr); _edgeColls.emplace_back(it); _graphInfo.add(VPackValue(it->name())); } _graphInfo.close(); for (auto& it : vertexColls) { + TRI_ASSERT(it != nullptr); addVertexCollection(*it); } } @@ -547,6 +549,7 @@ void GraphNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const { { VPackArrayBuilder guard(&nodes); for (auto const& e : _edgeColls) { + TRI_ASSERT(e != nullptr); auto const& shard = collectionToShardName(e->name()); // if the mapped shard for a collection is empty, it means that // we have an edge collection that is only relevant on some of the @@ -561,6 +564,7 @@ void GraphNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const { { VPackArrayBuilder guard(&nodes); for (auto const& v : _vertexColls) { + TRI_ASSERT(v != nullptr); // if the mapped shard for a collection is empty, it means that // we have a vertex collection that is only relevant on some of the // target servers @@ -631,6 +635,7 @@ CostEstimate GraphNode::estimateCost() const { double baseCost = 1; size_t baseNumItems = 0; for (auto& e : _edgeColls) { + TRI_ASSERT(e != nullptr); auto count = e->count(_options->trx(), transaction::CountType::TryCache); // Assume an estimate if 10% hit rate baseCost *= count / 10; @@ -794,9 +799,11 @@ std::vector GraphNode::collections() const { set.reserve(_edgeColls.size() + _vertexColls.size()); for (auto const& collPointer : _edgeColls) { + TRI_ASSERT(collPointer != nullptr); set.emplace(collPointer); } for (auto const& collPointer : _vertexColls) { + TRI_ASSERT(collPointer != nullptr); set.emplace(collPointer); } diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 8050a8239d54..2b01f8d6f311 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -63,7 +63,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, if (errorCode != TRI_ERROR_NO_ERROR) { THROW_ARANGO_EXCEPTION(errorCode); } - restrictedShards.emplace(shardId); + restrictedShards.emplace(std::move(shardId)); }; // If we have ever accessed the server lists, @@ -342,6 +342,7 @@ std::unordered_map const& ShardLocking::getShardMapping() { // We have at least one shard, otherwise we would not have snippets! TRI_ASSERT(!shardIds.empty()); _shardMapping = ci.getResponsibleServers(shardIds); + TRI_ASSERT(_shardMapping.size() == shardIds.size()); for (auto const& lockInfo : _collectionLocking) { for (auto const& sid : lockInfo.second.allShards) { @@ -352,6 +353,7 @@ std::unordered_map const& ShardLocking::getShardMapping() { } } } + return _shardMapping; } From 55324849f15b775dcf985c5479649794dd13570d Mon Sep 17 00:00:00 2001 From: jsteemann Date: Thu, 26 Aug 2021 00:32:40 +0200 Subject: [PATCH 3/9] added CHANGELOG entry --- CHANGELOG | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index f98165c8cccf..6b09eeb27333 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,36 @@ devel ----- +* (Enterprise Edition only): added query option `forceOneShardAttributeValue` + to explicitly set a shard key value that will be used during query snippet + distribution to limit the query to a specific server in the cluster. + + This query option can be used in complex queries in case the query optimizer + cannot automatically detect that the query can be limited to only a single + server (e.g. in a disjoint smart graph case). + When the option is set to the correct shard key value, the query will be + limited to the target server determined by the shard key value. It thus + requires that all collections in the query use the same distribution + (i.e. `distributeShardsLike` attribute via disjoint SmartGraphs). + + Limiting the query to a single DB server is a performance optimization + and may make complex queries run a lot faster because of the reduced + setup and teardown costs and the reduced cluster-internal traffic during + query execution. + + If the option is set incorrectly, i.e. to a wrong shard key value, then + the query may be shipped to a wrong DB server and may not return results + (i.e. empty result set). It is thus the caller's responsibility to set + the `forceOneShardAttributeValue` correctly or not use it. + + The `forceOneShardAttributeValue` option will only honor string values. + All other values as well as the empty string will be ignored and treated + as if the option is not set. + + If the option is set and the query satisfies the requirements for using + the option, the query's execution plan will contain the "cluster-one-shard" + optimizer rule. + * Fix a potential multi-threading issue in index creation on coordinators, when an agency callback was triggered at the same time the method `ensureIndexCoordinatorInner` was left. From 95ba26c7612948eeafd7514572e6bdc961ee997c Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 1 Sep 2021 14:08:57 +0200 Subject: [PATCH 4/9] only enable restrictedShards in case one shard rule got active --- arangod/Aql/ShardLocking.cpp | 41 +++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 2b01f8d6f311..53ae32b441e1 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -31,6 +31,7 @@ #include "Aql/GraphNode.h" #include "Aql/IResearchViewNode.h" #include "Aql/ModificationNodes.h" +#include "Aql/OptimizerRule.h" #include "Aql/Query.h" #include "Cluster/ClusterFeature.h" #include "Logger/LogMacros.h" @@ -45,20 +46,26 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, bool pushToSingleServer) { TRI_ASSERT(baseNode != nullptr); - std::string const& forceOneShardAttributeValue = _query.queryOptions().forceOneShardAttributeValue; + std::string const& forceOneShardAttributeValue = + _query.queryOptions().forceOneShardAttributeValue; + bool isOneShardEnabled = baseNode->plan()->hasAppliedRule(OptimizerRule::clusterOneShardRule); + bool useRestrictedShard = isOneShardEnabled && !forceOneShardAttributeValue.empty(); - auto addRestrictedShard = [&](aql::Collection const* col, + auto addRestrictedShard = [&](aql::Collection const* col, std::unordered_set& restrictedShards) { TRI_ASSERT(!forceOneShardAttributeValue.empty()); + TRI_ASSERT(useRestrictedShard); std::string shardId; auto errorCode = TRI_ERROR_NO_ERROR; if (col->isDisjoint()) { - // if disjoint smart edge collection, we must insert an + // if disjoint smart edge collection, we must insert an // artifical key with two colons, to pretend it is a real // smart graph key - errorCode = col->getCollection()->getResponsibleShard(forceOneShardAttributeValue + ":test:" + forceOneShardAttributeValue, shardId); + errorCode = col->getCollection()->getResponsibleShard( + forceOneShardAttributeValue + ":test:" + forceOneShardAttributeValue, shardId); } else { - errorCode = col->getCollection()->getResponsibleShard(forceOneShardAttributeValue, shardId); + errorCode = + col->getCollection()->getResponsibleShard(forceOneShardAttributeValue, shardId); } if (errorCode != TRI_ERROR_NO_ERROR) { THROW_ARANGO_EXCEPTION(errorCode); @@ -90,20 +97,22 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, // Add all Edge Collections to the Transactions, Traversals do never write for (auto const& col : graphNode->edgeColls()) { std::unordered_set restrictedShards; - if (!forceOneShardAttributeValue.empty()) { + if (useRestrictedShard) { addRestrictedShard(col, restrictedShards); } - updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, isUsedAsSatellite(col)); + updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, + isUsedAsSatellite(col)); } // Add all Vertex Collections to the Transactions, Traversals do never // write, the collections have been adjusted already for (auto const& col : graphNode->vertexColls()) { std::unordered_set restrictedShards; - if (!forceOneShardAttributeValue.empty()) { + if (useRestrictedShard) { addRestrictedShard(col, restrictedShards); } - updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, isUsedAsSatellite(col)); + updateLocking(col, AccessMode::Type::READ, snippetId, restrictedShards, + isUsedAsSatellite(col)); } break; } @@ -116,7 +125,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, "unable to cast node to CollectionAccessingNode"); } std::unordered_set restrictedShards; - if (!forceOneShardAttributeValue.empty()) { + if (useRestrictedShard) { addRestrictedShard(colNode->collection(), restrictedShards); } else if (colNode->isRestricted()) { restrictedShards.emplace(colNode->restrictedShard()); @@ -135,7 +144,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, } for (aql::Collection const& col : viewNode->collections()) { std::unordered_set restrictedShards; - if (!forceOneShardAttributeValue.empty()) { + if (useRestrictedShard) { addRestrictedShard(&col, restrictedShards); } updateLocking(&col, AccessMode::Type::READ, snippetId, restrictedShards, false); @@ -156,7 +165,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, auto* col = modNode->collection(); std::unordered_set restrictedShards; - if (!forceOneShardAttributeValue.empty()) { + if (useRestrictedShard) { addRestrictedShard(modNode->collection(), restrictedShards); } else if (modNode->isRestricted()) { restrictedShards.emplace(modNode->restrictedShard()); @@ -193,11 +202,13 @@ void ShardLocking::updateLocking(Collection const* col, auto const& name = col->name(); if (!TRI_vocbase_t::IsSystemName(name)) { LOG_TOPIC("0997e", WARN, arangodb::Logger::AQL) - << "Accessing collection: " << name << " does not translate to any shard. Aborting query."; + << "Accessing collection: " << name + << " does not translate to any shard. Aborting query."; } THROW_ARANGO_EXCEPTION_MESSAGE( TRI_ERROR_QUERY_COLLECTION_LOCK_FAILED, - "Could not identify any shard belonging to collection: " + name + ". Maybe it is dropped?"); + "Could not identify any shard belonging to collection: " + name + + ". Maybe it is dropped?"); } for (auto const& s : *shards) { info.allShards.emplace(s); @@ -353,7 +364,7 @@ std::unordered_map const& ShardLocking::getShardMapping() { } } } - + return _shardMapping; } From a198e5fb7b0b8ac6a2da18e7f28c42107caf8b63 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 1 Sep 2021 15:59:07 +0200 Subject: [PATCH 5/9] fixed getResponsibleShards usage --- arangod/Aql/ShardLocking.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 53ae32b441e1..de073f2129ba 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -64,8 +64,24 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, errorCode = col->getCollection()->getResponsibleShard( forceOneShardAttributeValue + ":test:" + forceOneShardAttributeValue, shardId); } else { - errorCode = - col->getCollection()->getResponsibleShard(forceOneShardAttributeValue, shardId); + auto const& shardKeys = col->getCollection().get()->shardKeys(); + TRI_ASSERT(shardKeys.size()); + auto const& shardKey = shardKeys.at(0); + if (shardKey == "_key:") { + errorCode = + col->getCollection()->getResponsibleShard(forceOneShardAttributeValue + ":test", shardId); + } else if (shardKey == ":_key") { + errorCode = + col->getCollection()->getResponsibleShard("test:" + forceOneShardAttributeValue, shardId); + } else { + VPackBuilder builder; + { + VPackObjectBuilder guard(&builder); + builder.add(shardKey, VPackValue(forceOneShardAttributeValue)); + } + errorCode = + col->getCollection()->getResponsibleShard(builder.slice(), false, shardId); + } } if (errorCode != TRI_ERROR_NO_ERROR) { THROW_ARANGO_EXCEPTION(errorCode); From b27c9bf1f6939cd7365b61d5cc422349ae653743 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 3 Sep 2021 11:54:18 +0200 Subject: [PATCH 6/9] do not check single boolean twice :) --- arangod/Aql/ShardLocking.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index de073f2129ba..801b09e5b80d 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -48,8 +48,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, std::string const& forceOneShardAttributeValue = _query.queryOptions().forceOneShardAttributeValue; - bool isOneShardEnabled = baseNode->plan()->hasAppliedRule(OptimizerRule::clusterOneShardRule); - bool useRestrictedShard = isOneShardEnabled && !forceOneShardAttributeValue.empty(); + bool useRestrictedShard = pushToSingleServer && !forceOneShardAttributeValue.empty(); auto addRestrictedShard = [&](aql::Collection const* col, std::unordered_set& restrictedShards) { From 595fce7ce4679eb2a2efbd9a0d30bab8541f35f0 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 6 Sep 2021 15:50:03 +0200 Subject: [PATCH 7/9] changelog --- CHANGELOG | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 34bf96c3433c..1f7018169e06 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,16 @@ devel ----- +* (EE only) Bug-fix: If you created a ArangoSearch view on Satellite- + Collections only and then join with a collection only having a single + shard the cluster-one-shard-rule was falsly applied and could lead to + empty view results. The Rule will now detect the situation properly, + and not trigger. + +* (EE only) If you have a query using only satellite collections, + now the cluster-one-shard-rule can be applied to improve + query performance. + * (Enterprise Edition only): added query option `forceOneShardAttributeValue` to explicitly set a shard key value that will be used during query snippet distribution to limit the query to a specific server in the cluster. From f7c29b69a8164b21a4e5bba53640a424da275b20 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 6 Sep 2021 15:51:11 +0200 Subject: [PATCH 8/9] changelog --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 1f7018169e06..a91b8a07446c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,7 +3,7 @@ devel * (EE only) Bug-fix: If you created a ArangoSearch view on Satellite- Collections only and then join with a collection only having a single - shard the cluster-one-shard-rule was falsly applied and could lead to + shard the cluster-one-shard-rule was falsely applied and could lead to empty view results. The Rule will now detect the situation properly, and not trigger. From 2c70645bb0ca141e1b3ab839ade176e5290b2147 Mon Sep 17 00:00:00 2001 From: Jan Date: Tue, 7 Sep 2021 10:39:18 +0200 Subject: [PATCH 9/9] Update arangod/Aql/ShardLocking.cpp Co-authored-by: Michael Hackstein --- arangod/Aql/ShardLocking.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 801b09e5b80d..a71ec6549c13 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -64,7 +64,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId, forceOneShardAttributeValue + ":test:" + forceOneShardAttributeValue, shardId); } else { auto const& shardKeys = col->getCollection().get()->shardKeys(); - TRI_ASSERT(shardKeys.size()); + TRI_ASSERT(!shardKeys.empty()); auto const& shardKey = shardKeys.at(0); if (shardKey == "_key:") { errorCode =