arangodb · jsteemann · Sep 7, 2021 · Aug 11, 2021 · Aug 19, 2021 · Aug 23, 2021
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,46 @@
 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 falsely 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.
+
+  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.
+
 * Updated ArangoDB Starter to 0.15.2.
 
 * SEARCH-238: Improved SortNodes placement optimization in cluster so 

diff --git 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<ShardID> EngineInfoContainerDBServerServerBased::TraverserEngineShar
   std::vector<ShardID> 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,15 +859,15 @@ 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"));
   // This will open and close an Object.
   _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<ExecutionNodeId, ExecutionNode*> 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<bool> EngineInfoContainerDBServerServerBased::addTraversalEnginesPart(
     arangodb::velocypack::Builder& infoBuilder,
     std::unordered_map<ShardID, ServerID> const& shardMapping, ServerID const& server) const {

diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp
@@ -466,12 +466,14 @@ void GraphNode::setGraphInfoAndCopyColls(std::vector<Collection*> const& edgeCol
                                          std::vector<Collection*> 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<aql::Collection const*> 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/QueryOptions.cpp b/arangod/Aql/QueryOptions.cpp
@@ -172,50 +172,43 @@ void QueryOptions::fromVelocyPack(VPackSlice slice) {
     traversalProfile = static_cast<TraversalProfileLevel>(value.getNumber<uint16_t>());
   }
 
-  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
@@ -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<std::string> optimizerRules;