From 425c005efd1ca89111a835f405bd64e82ed00734 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Mon, 6 Jan 2020 18:29:22 +0100 Subject: [PATCH 01/47] added replicationFactor satellite --- arangod/Graph/Graph.cpp | 59 ++++++++++++++++++++++++++++++----------- arangod/Graph/Graph.h | 2 ++ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/arangod/Graph/Graph.cpp b/arangod/Graph/Graph.cpp index ab035c2b6f57..e3a0db1f9d8a 100644 --- a/arangod/Graph/Graph.cpp +++ b/arangod/Graph/Graph.cpp @@ -22,6 +22,7 @@ #include "Graph.h" +#include #include #include #include @@ -80,20 +81,18 @@ size_t getWriteConcern(VPackSlice slice) { } return Helper::getNumericValue(slice, StaticStrings::MinReplicationFactor, 1); } -} +} // namespace // From persistence Graph::Graph(velocypack::Slice const& slice) : _graphName(Helper::getStringValue(slice, StaticStrings::KeyString, "")), _vertexColls(), _edgeColls(), - _numberOfShards(Helper::getNumericValue(slice, StaticStrings::NumberOfShards, - 1)), - _replicationFactor(Helper::getNumericValue( - slice, StaticStrings::ReplicationFactor, 1)), + _numberOfShards(Helper::getNumericValue(slice, StaticStrings::NumberOfShards, 1)), + _replicationFactor( + Helper::getNumericValue(slice, StaticStrings::ReplicationFactor, 1)), _writeConcern(::getWriteConcern(slice)), - _rev(Helper::getStringValue(slice, StaticStrings::RevString, - "")) { + _rev(Helper::getStringValue(slice, StaticStrings::RevString, "")) { // If this happens we have a document without an _key Attribute. if (_graphName.empty()) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, @@ -117,6 +116,11 @@ Graph::Graph(velocypack::Slice const& slice) if (slice.hasKey(StaticStrings::GraphOrphans)) { insertOrphanCollections(slice.get(StaticStrings::GraphOrphans)); } + if (slice.hasKey(StaticStrings::ReplicationFactor) && + slice.get(StaticStrings::ReplicationFactor).isString() && + slice.get(StaticStrings::ReplicationFactor).isEqualString(StaticStrings::Satellite)) { + setReplicationFactor(0); + } } // From user input @@ -140,9 +144,16 @@ Graph::Graph(std::string&& graphName, VPackSlice const& info, VPackSlice const& insertOrphanCollections(info.get(StaticStrings::GraphOrphans)); } if (options.isObject()) { - _numberOfShards = Helper::getNumericValue(options, StaticStrings::NumberOfShards, 1); - _replicationFactor = Helper::getNumericValue(options, StaticStrings::ReplicationFactor, 1); - _writeConcern = ::getWriteConcern(options); + _numberOfShards = + Helper::getNumericValue(options, StaticStrings::NumberOfShards, 1); + if (Helper::getStringValue(options.get(StaticStrings::ReplicationFactor), + "1") == StaticStrings::Satellite) { + setReplicationFactor(0); + } else { + _replicationFactor = + Helper::getNumericValue(options, StaticStrings::ReplicationFactor, 1); + _writeConcern = ::getWriteConcern(options); + } } } @@ -303,9 +314,13 @@ void Graph::toPersistence(VPackBuilder& builder) const { // Cluster Information builder.add(StaticStrings::NumberOfShards, VPackValue(_numberOfShards)); - builder.add(StaticStrings::ReplicationFactor, VPackValue(_replicationFactor)); - builder.add(StaticStrings::MinReplicationFactor, VPackValue(_writeConcern)); // deprecated - builder.add(StaticStrings::WriteConcern, VPackValue(_writeConcern)); + if (isSatellite()) { + builder.add(StaticStrings::ReplicationFactor, VPackValue(StaticStrings::Satellite)); + } else { + builder.add(StaticStrings::ReplicationFactor, VPackValue(_replicationFactor)); + builder.add(StaticStrings::MinReplicationFactor, VPackValue(_writeConcern)); // deprecated + builder.add(StaticStrings::WriteConcern, VPackValue(_writeConcern)); + } builder.add(StaticStrings::GraphIsSmart, VPackValue(isSmart())); // EdgeDefinitions @@ -679,17 +694,29 @@ void Graph::verticesToVpack(VPackBuilder& builder) const { bool Graph::isSmart() const { return false; } +bool Graph::isSatellite() const { + if (replicationFactor() == 0) { + return true; + } + return false; +} + void Graph::createCollectionOptions(VPackBuilder& builder, bool waitForSync) const { TRI_ASSERT(builder.isOpenObject()); builder.add(StaticStrings::WaitForSyncString, VPackValue(waitForSync)); builder.add(StaticStrings::NumberOfShards, VPackValue(numberOfShards())); + + if (!isSatellite()) { + builder.add(StaticStrings::MinReplicationFactor, VPackValue(writeConcern())); // deprecated + builder.add(StaticStrings::WriteConcern, VPackValue(writeConcern())); + } + builder.add(StaticStrings::ReplicationFactor, VPackValue(replicationFactor())); - builder.add(StaticStrings::MinReplicationFactor, VPackValue(writeConcern())); // deprecated - builder.add(StaticStrings::WriteConcern, VPackValue(writeConcern())); } -std::optional> Graph::getEdgeDefinition(std::string const& collectionName) const { +std::optional> Graph::getEdgeDefinition( + std::string const& collectionName) const { auto it = edgeDefinitions().find(collectionName); if (it == edgeDefinitions().end()) { TRI_ASSERT(!hasEdgeCollection(collectionName)); diff --git a/arangod/Graph/Graph.h b/arangod/Graph/Graph.h index 6f29a36b8275..eea9050a079a 100644 --- a/arangod/Graph/Graph.h +++ b/arangod/Graph/Graph.h @@ -165,6 +165,7 @@ class Graph { std::optional> getEdgeDefinition(std::string const& collectionName) const; virtual bool isSmart() const; + virtual bool isSatellite() const; uint64_t numberOfShards() const; uint64_t replicationFactor() const; @@ -287,6 +288,7 @@ class Graph { uint64_t _numberOfShards; /// @brief replication factor of this graph + /// if the value is set to 0, it will be threaten as a satellite graph uint64_t _replicationFactor; /// @brief write concern for this graph From 555ca78da7e0be28e38d048af5051e9365443e13 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Mon, 6 Jan 2020 18:56:45 +0100 Subject: [PATCH 02/47] added method to check if graph exists only out of satellite collections --- arangod/Graph/GraphManager.cpp | 24 ++++++++++++++++++++++++ arangod/Graph/GraphManager.h | 3 +++ 2 files changed, 27 insertions(+) diff --git a/arangod/Graph/GraphManager.cpp b/arangod/Graph/GraphManager.cpp index 0c3abe331b0e..28da3aa0c2e1 100644 --- a/arangod/Graph/GraphManager.cpp +++ b/arangod/Graph/GraphManager.cpp @@ -598,6 +598,30 @@ Result GraphManager::ensureCollections(Graph const* graph, bool waitForSync) con vocbase, collectionsToCreate, waitForSync, true, false, nullptr, created); }; +bool GraphManager::onlySatellitesUsed(Graph const* graph) const { + bool onlySatellites = true; + + for (auto const& cname : graph->vertexCollections()) { + if (!_vocbase.lookupCollection(cname).get()->isSatellite()) { + onlySatellites = false; + } + if (!onlySatellites) { + break; // quick exit + } + } + + for (auto const& cname : graph->edgeCollections()) { + if (!_vocbase.lookupCollection(cname).get()->isSatellite()) { + onlySatellites = false; + } + if (!onlySatellites) { + break; // quick exit + } + } + + return onlySatellites; +}; + OperationResult GraphManager::readGraphs(velocypack::Builder& builder, aql::QueryPart const queryPart) const { std::string const queryStr{ diff --git a/arangod/Graph/GraphManager.h b/arangod/Graph/GraphManager.h index 62217f6c2eff..18ae47ca1d13 100644 --- a/arangod/Graph/GraphManager.h +++ b/arangod/Graph/GraphManager.h @@ -155,6 +155,9 @@ class GraphManager { */ Result ensureCollections(Graph const* graph, bool waitForSync) const; + /// @brief check if only satellite collections are used + bool onlySatellitesUsed(Graph const* graph) const; + /** * @brief Store the given graph * From bf150c036dbe261700f0b8be9c80a25593febf88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Tue, 28 Jan 2020 12:26:01 +0100 Subject: [PATCH 03/47] Added SatelliteTraversalNode --- arangod/Aql/GraphNode.cpp | 13 +++++--- arangod/Aql/GraphNode.h | 7 ++-- arangod/Aql/SatelliteTraversalNode.cpp | 45 ++++++++++++++++++++++++++ arangod/Aql/SatelliteTraversalNode.h | 37 +++++++++++++++++++++ arangod/Aql/TraversalNode.cpp | 41 +++++++++++++++++++++++ arangod/Aql/TraversalNode.h | 5 +++ arangod/CMakeLists.txt | 1 + 7 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 arangod/Aql/SatelliteTraversalNode.cpp create mode 100644 arangod/Aql/SatelliteTraversalNode.h diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index c367f8c23235..9b8e782ec40d 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -31,7 +31,6 @@ #include "Aql/Ast.h" #include "Aql/Collection.h" #include "Aql/ExecutionPlan.h" -#include "Aql/Query.h" #include "Cluster/ClusterFeature.h" #include "Cluster/ServerState.h" #include "Graph/BaseOptions.h" @@ -39,6 +38,8 @@ #include "Utils/CollectionNameResolver.h" #include "VocBase/LogicalCollection.h" +#include + using namespace arangodb; using namespace arangodb::aql; using namespace arangodb::graph; @@ -385,7 +386,7 @@ GraphNode::GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& bas GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, - std::vector const& directions, + std::vector directions, std::unique_ptr options) : ExecutionNode(plan, id), _vocbase(vocbase), @@ -395,7 +396,7 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, _tmpObjVariable(_plan->getAst()->variables()->createTemporaryVariable()), _tmpObjVarNode(_plan->getAst()->createNodeReference(_tmpObjVariable)), _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), - _directions(directions), + _directions(std::move(directions)), _options(std::move(options)), _optionsBuilt(false), _isSmart(false) { @@ -417,8 +418,6 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, } } -GraphNode::~GraphNode() = default; - std::string const& GraphNode::collectionToShardName(std::string const& collName) const { if (_collectionToShard.empty()) { return collName; @@ -654,3 +653,7 @@ std::vector> const& GraphNode::edgeColls() cons std::vector> const& GraphNode::vertexColls() const { return _vertexColls; } + +graph::Graph const* GraphNode::graph() const noexcept { + return _graphObj; +} diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 8786c72b6444..8aa10bf07d7b 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -64,13 +64,13 @@ class GraphNode : public ExecutionNode { GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, - std::vector const& directions, + std::vector directions, std::unique_ptr options); std::string const& collectionToShardName(std::string const& collName) const; public: - ~GraphNode() override; + ~GraphNode() override = default; void toVelocyPackHelper(arangodb::velocypack::Builder& nodes, unsigned flags, std::unordered_set& seen) const override; @@ -141,6 +141,9 @@ class GraphNode : public ExecutionNode { void setCollectionToShard(std::map const& map) { _collectionToShard = map; } void addCollectionToShard(std::string const& coll, std::string const& shard) { _collectionToShard.emplace(coll,shard); } + protected: + graph::Graph const* graph() const noexcept; + private: void addEdgeCollection(std::string const& n, TRI_edge_direction_e dir); diff --git a/arangod/Aql/SatelliteTraversalNode.cpp b/arangod/Aql/SatelliteTraversalNode.cpp new file mode 100644 index 000000000000..aef34cf5364a --- /dev/null +++ b/arangod/Aql/SatelliteTraversalNode.cpp @@ -0,0 +1,45 @@ +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2020 ArangoDB GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is ArangoDB GmbH, Cologne, Germany +/// +/// @author Tobias Gödderz +//////////////////////////////////////////////////////////////////////////////// + +#include "SatelliteTraversalNode.h" + +#include "Basics/Exceptions.h" +#include "Graph/Graph.h" + +using namespace arangodb; +using namespace arangodb::aql; + +SatelliteTraversalNode::SatelliteTraversalNode(TraversalNode&& traversalNode, + aql::Collection const& collection) + : TraversalNode(std::move(traversalNode)), CollectionAccessingNode(&collection) { + // TODO Replace the following checks with a check on traversalNode->isEligibleAsSatelliteTraversal(). + if (graph() == nullptr) { + TRI_ASSERT(false); + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL_AQL, "Logic error: satellite traversals currently only supported on named graphs"); + } + + if (!graph()->isSatellite()) { + TRI_ASSERT(false); + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL_AQL, "Logic error: satellite traversals on non-satellite graph"); + } + +} diff --git a/arangod/Aql/SatelliteTraversalNode.h b/arangod/Aql/SatelliteTraversalNode.h new file mode 100644 index 000000000000..7a28ebb45c77 --- /dev/null +++ b/arangod/Aql/SatelliteTraversalNode.h @@ -0,0 +1,37 @@ +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2020 ArangoDB GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is ArangoDB GmbH, Cologne, Germany +/// +/// @author Tobias Gödderz +//////////////////////////////////////////////////////////////////////////////// + +#ifndef ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H +#define ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H + +#include "Aql/CollectionAccessingNode.h" +#include "Aql/TraversalNode.h" + +namespace arangodb::aql { + +class SatelliteTraversalNode : public TraversalNode, public CollectionAccessingNode { + SatelliteTraversalNode(TraversalNode&& traversalNode, aql::Collection const& collection); +}; + +} // namespace arangodb::aql + +#endif // ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index 800cd0d9294d..5187d984f4f0 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -173,6 +173,47 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocb _fromCondition(nullptr), _toCondition(nullptr) {} +TraversalNode::TraversalNode(TraversalNode&& other) + : GraphNode(other._plan, other._id, other._vocbase, other._edgeColls, other._vertexColls, + std::move(other._directions), std::move(other._options)), + _pathOutVariable(other._pathOutVariable), + _inVariable(other._inVariable), + _vertexId(std::move(other._vertexId)), + _condition(std::move(other._condition)), + _conditionVariables(std::move(other._conditionVariables)), + _fromCondition(other._fromCondition), + _toCondition(other._toCondition), + _pruneExpression(std::move(other._pruneExpression)), + _globalEdgeConditions(std::move(other._globalEdgeConditions)), + _globalVertexConditions(std::move(other._globalVertexConditions)), + _edgeConditions(std::move(other._edgeConditions)), + _vertexConditions(std::move(other._vertexConditions)), + _pruneVariables(std::move(other._pruneVariables)) { + // TODO Maybe we need to copy/move members of ExecutionNode as well! + + // Copy remaining members of GraphNode + _vertexOutVariable = other._vertexOutVariable; + _edgeOutVariable = other._edgeOutVariable; + _graphObj = other._graphObj; + _defaultDirection = other._defaultDirection; + _isSmart = other._isSmart; + + // Clear everything that's not moved, especially everything involving manual + // memory management. + other._plan = nullptr; + other._id = -1; + other._vocbase = nullptr; + other._edgeColls.clear(); + other._vertexColls.clear(); + other._pathOutVariable = nullptr; + other._inVariable = nullptr; + other._fromCondition = nullptr; + other._toCondition = nullptr; + other._vertexOutVariable = nullptr; + other._edgeOutVariable = nullptr; + other._graphObj = nullptr; +} + TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base) : GraphNode(plan, base), _pathOutVariable(nullptr), diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index 90b30d17260b..70b4943c6aca 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -93,6 +93,11 @@ class TraversalNode : public GraphNode { std::vector const& directions, std::unique_ptr options); + protected: + /// @brief Move from another traversal node. Used to create a + /// SatelliteTraversalNode from a TraversalNode, see the constructor there. + TraversalNode(TraversalNode&& other); + public: /// @brief return the type of the node NodeType getType() const override final { return TRAVERSAL; } diff --git a/arangod/CMakeLists.txt b/arangod/CMakeLists.txt index b6c0e1d7be95..727d8ef4f401 100644 --- a/arangod/CMakeLists.txt +++ b/arangod/CMakeLists.txt @@ -329,6 +329,7 @@ set(LIB_ARANGO_AQL_SOURCES Aql/RemoteExecutor.cpp Aql/RestAqlHandler.cpp Aql/ReturnExecutor.cpp + Aql/SatelliteTraversalNode.cpp Aql/ScatterExecutor.cpp Aql/Scopes.cpp Aql/ShadowAqlItemRow.cpp From 872721b740ac68fb1dda22bb155f838e462e5490 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 13:54:29 +0100 Subject: [PATCH 04/47] added scatterSatelliteGraphRule optimizer rule, still wip --- arangod/Aql/OptimizerRule.h | 7 +- arangod/Aql/OptimizerRules.cpp | 184 ++++++++++---------------- arangod/Aql/OptimizerRules.h | 75 +++++++++++ arangod/Aql/OptimizerRulesFeature.cpp | 5 + 4 files changed, 154 insertions(+), 117 deletions(-) diff --git a/arangod/Aql/OptimizerRule.h b/arangod/Aql/OptimizerRule.h index fbf3546a35fd..4330f62839dd 100644 --- a/arangod/Aql/OptimizerRule.h +++ b/arangod/Aql/OptimizerRule.h @@ -270,6 +270,9 @@ struct OptimizerRule { removeUnnecessaryRemoteScatterRule, #ifdef USE_ENTERPRISE + // move traversal on satellite graph to db server and add scatter / gather / remote + scatterSatelliteGraphRule, + // remove any superflous satellite collection joins... // put it after Scatter rule because we would do // the work twice otherwise @@ -321,12 +324,12 @@ struct OptimizerRule { static_assert(clusterOneShardRule < distributeInClusterRule); static_assert(clusterOneShardRule < smartJoinsRule); static_assert(clusterOneShardRule < scatterInClusterRule); - + // smart joins must come before we move filters around, so the smart-join // detection code does not need to take the special filters into account static_assert(smartJoinsRule < moveFiltersIntoEnumerateRule); #endif - + static_assert(scatterInClusterRule < parallelizeGatherRule); velocypack::StringRef name; diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 9f3fb65f9c4c..79ef76af4f82 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -600,10 +600,9 @@ std::vector const patchUpdateRemoveState arangodb::aql::ExecutionNode::UPDATE, arangodb::aql::ExecutionNode::REPLACE, arangodb::aql::ExecutionNode::REMOVE}; std::vector const moveFilterIntoEnumerateTypes{ - arangodb::aql::ExecutionNode::ENUMERATE_COLLECTION, - arangodb::aql::ExecutionNode::INDEX}; + arangodb::aql::ExecutionNode::ENUMERATE_COLLECTION, arangodb::aql::ExecutionNode::INDEX}; std::vector const undistributeNodeTypes{ - arangodb::aql::ExecutionNode::UPDATE, arangodb::aql::ExecutionNode::REPLACE, + arangodb::aql::ExecutionNode::UPDATE, arangodb::aql::ExecutionNode::REPLACE, arangodb::aql::ExecutionNode::REMOVE}; /// @brief find the single shard id for the node to restrict an operation to @@ -1494,7 +1493,7 @@ class PropagateConstantAttributesHelper { if (it == _constants.end()) { _constants.try_emplace(variable, - std::unordered_map{{name, value}}); + std::unordered_map{{name, value}}); return; } @@ -2772,7 +2771,7 @@ void arangodb::aql::removeUnnecessaryCalculationsRule(Optimizer* opt, // no COLLECT found, now replace std::unordered_map replacements; replacements.try_emplace(outVariable->id, - static_cast(rootNode->getData())); + static_cast(rootNode->getData())); RedundantCalculationsReplacer finder(plan->getAst(), replacements); plan->root()->walk(finder); @@ -3093,7 +3092,7 @@ struct SortToIndexNode final : public WalkerWorker { bool isSorted = index->isSorted(); bool isSparse = index->sparse(); std::vector> fields = index->fields(); - + if (indexes.size() != 1) { // can only use this index node if it uses exactly one index or multiple // indexes on exactly the same attributes @@ -3615,6 +3614,7 @@ void arangodb::aql::interchangeAdjacentEnumerationsRule(Optimizer* opt, opt->addPlan(std::move(plan), rule, false); } + /// @brief scatter operations in cluster /// this rule inserts scatter, gather and remote nodes so operations on sharded /// collections actually work @@ -3649,7 +3649,6 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrgetParents(); // intentional copy of the dependencies, as we will be modifying // dependencies later on auto const deps = node->getDependencies(); @@ -3665,7 +3664,6 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrisRoot(node); plan->unlinkNode(node, true); auto const nodeType = node->getType(); @@ -3722,60 +3720,8 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrnextId(), ScatterNode::ScatterType::SHARD); - plan->registerNode(scatterNode); - TRI_ASSERT(!deps.empty()); - scatterNode->addDependency(deps[0]); - - // insert a remote node - ExecutionNode* remoteNode = - new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); - plan->registerNode(remoteNode); - TRI_ASSERT(scatterNode); - remoteNode->addDependency(scatterNode); - - // re-link with the remote node - node->addDependency(remoteNode); - - // insert another remote node - remoteNode = new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); - plan->registerNode(remoteNode); - TRI_ASSERT(node); - remoteNode->addDependency(node); - - // insert a gather node - auto const sortMode = GatherNode::evaluateSortMode(collection->numberOfShards()); - // single-sharded collections don't require any parallelism. collections with more than - // one shard are eligible for later parallelization (the Undefined allows this) - auto const parallelism = (collection->numberOfShards() <= 1 ? GatherNode::Parallelism::Serial : GatherNode::Parallelism::Undefined); - auto* gatherNode = new GatherNode(plan.get(), plan->nextId(), sortMode, parallelism); - plan->registerNode(gatherNode); - TRI_ASSERT(remoteNode); - gatherNode->addDependency(remoteNode); - // On SmartEdge collections we have 0 shards and we need the elements - // to be injected here as well. So do not replace it with > 1 - if (!elements.empty() && collection->numberOfShards() != 1) { - gatherNode->elements(elements); - } - - // and now link the gather node with the rest of the plan - if (parents.size() == 1) { - parents[0]->replaceDependency(deps[0], gatherNode); - } - - // check if the node that we modified was at the end of a subquery - auto it = subqueries.find(node); - - if (it != subqueries.end()) { - ExecutionNode::castTo((*it).second)->setSubquery(gatherNode, true); - } - - if (isRootNode) { - // if we replaced the root node, set a new root node - plan->root(gatherNode); - } + size_t numberOfShards = collection->numberOfShards(); + plan = scatterHelperFunction(std::move(plan), vocbase, node, elements, numberOfShards, subqueries); wasModified = true; } @@ -3960,8 +3906,8 @@ void arangodb::aql::distributeInClusterRule(Optimizer* opt, // an UPSERT node has two input variables! auto upsertNode = ExecutionNode::castTo(node); auto d = new DistributeNode(plan.get(), plan->nextId(), - ScatterNode::ScatterType::SHARD, - collection, upsertNode->inDocVariable(), + ScatterNode::ScatterType::SHARD, collection, + upsertNode->inDocVariable(), upsertNode->insertVariable(), true, true); d->setAllowSpecifiedKeys(true); distNode = ExecutionNode::castTo(d); @@ -4846,8 +4792,8 @@ class RemoveToEnumCollFinder final : public WalkerWorker { bool before(ExecutionNode* en) override final { switch (en->getType()) { - case EN::UPDATE: - case EN::REPLACE: + case EN::UPDATE: + case EN::REPLACE: case EN::REMOVE: { if (_foundModification) { break; @@ -4856,7 +4802,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { // find the variable we are removing . . . auto rn = ExecutionNode::castTo(en); Variable const* toRemove = nullptr; - + if (en->getType() == EN::REPLACE) { toRemove = ExecutionNode::castTo(en)->inKeyVariable(); } else if (en->getType() == EN::UPDATE) { @@ -4875,7 +4821,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { _setter = _plan->getVarSetBy(toRemove->id); TRI_ASSERT(_setter != nullptr); auto enumColl = _setter; - + if (_setter->getType() == EN::CALCULATION) { // this should be an attribute access for _key auto cn = ExecutionNode::castTo(_setter); @@ -4891,7 +4837,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { if (shardKeys.size() != 1 || shardKeys[0] != StaticStrings::KeyString) { break; // abort . . . } - + // set the varsToRemove to the variable in the expression of this // node and also define enumColl ::arangodb::containers::HashSet varsToRemove; @@ -4975,10 +4921,11 @@ class RemoveToEnumCollFinder final : public WalkerWorker { break; // abort . . . } - auto const& projections = dynamic_cast(enumColl)->projections(); - if (projections.size() > 1 || + auto const& projections = + dynamic_cast(enumColl)->projections(); + if (projections.size() > 1 || (!projections.empty() && projections[0] != StaticStrings::KeyString)) { - // cannot handle projections + // cannot handle projections break; } @@ -4999,7 +4946,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { case EN::DISTRIBUTE: case EN::SCATTER: { if (_foundScatter) { // met more than one scatter node - break; // abort . . . + break; // abort . . . } _foundScatter = true; _toUnlink.emplace(en); @@ -5007,7 +4954,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { } case EN::GATHER: { if (_foundGather) { // met more than one gather node - break; // abort . . . + break; // abort . . . } _foundGather = true; _toUnlink.emplace(en); @@ -5052,7 +4999,7 @@ class RemoveToEnumCollFinder final : public WalkerWorker { TRI_ASSERT(false); } } - + _toUnlink.clear(); return true; } @@ -5398,7 +5345,8 @@ void arangodb::aql::replaceOrWithInRule(Optimizer* opt, std::unique_ptrnextId(), std::move(expr), outVar); + ExecutionNode* newNode = + new CalculationNode(plan.get(), plan->nextId(), std::move(expr), outVar); plan->registerNode(newNode); plan->replaceNode(cn, newNode); @@ -5562,7 +5510,8 @@ void arangodb::aql::removeRedundantOrRule(Optimizer* opt, auto astNode = remover.createReplacementNode(plan->getAst()); auto expr = std::make_unique(plan.get(), plan->getAst(), astNode); - ExecutionNode* newNode = new CalculationNode(plan.get(), plan->nextId(), std::move(expr), outVar); + ExecutionNode* newNode = + new CalculationNode(plan.get(), plan->nextId(), std::move(expr), outVar); plan->registerNode(newNode); plan->replaceNode(cn, newNode); modified = true; @@ -6912,7 +6861,8 @@ void arangodb::aql::sortLimitRule(Optimizer* opt, std::unique_ptr bool mod = false; // If there isn't a limit node, and at least one sort or gather node, there's // nothing to do. - if (!plan->contains(EN::LIMIT) || (!plan->contains(EN::SORT) && !plan->contains(EN::GATHER))) { + if (!plan->contains(EN::LIMIT) || + (!plan->contains(EN::SORT) && !plan->contains(EN::GATHER))) { opt->addPlan(std::move(plan), rule, mod); return; } @@ -7031,10 +6981,10 @@ void arangodb::aql::optimizeSubqueriesRule(Optimizer* opt, if (found.first != nullptr) { auto it = subqueryAttributes.find(found.first); if (it == subqueryAttributes.end()) { - subqueryAttributes.try_emplace(found.first, - std::make_tuple(found.second, - std::unordered_set{n}, - usedForCount)); + subqueryAttributes.try_emplace( + found.first, std::make_tuple(found.second, + std::unordered_set{n}, + usedForCount)); } else { auto& sq = (*it).second; if (usedForCount) { @@ -7155,7 +7105,8 @@ void arangodb::aql::optimizeSubqueriesRule(Optimizer* opt, } /// @brief move filters into EnumerateCollection nodes -void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, std::unique_ptr plan, +void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, + std::unique_ptr plan, OptimizerRule const& rule) { bool modified = false; @@ -7168,12 +7119,14 @@ void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, std::unique_ptr for (auto const& n : nodes) { auto en = dynamic_cast(n); if (en == nullptr) { - THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "unable to cast node to DocumentProducingNode"); + THROW_ARANGO_EXCEPTION_MESSAGE( + TRI_ERROR_INTERNAL, "unable to cast node to DocumentProducingNode"); } - if (n->getType() == EN::INDEX && ExecutionNode::castTo(n)->getIndexes().size() != 1) { - // we can only handle exactly one index right now. otherwise some IndexExecutor code - // may assert and fail + if (n->getType() == EN::INDEX && + ExecutionNode::castTo(n)->getIndexes().size() != 1) { + // we can only handle exactly one index right now. otherwise some + // IndexExecutor code may assert and fail continue; } @@ -7197,7 +7150,7 @@ void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, std::unique_ptr if (calculations.empty()) { break; } - + auto filterNode = ExecutionNode::castTo(current); Variable const* inVariable = filterNode->inVariable(); @@ -7211,8 +7164,10 @@ void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, std::unique_ptr Expression* existingFilter = en->filter(); if (existingFilter != nullptr && existingFilter->node() != nullptr) { // node already has a filter, now AND-merge it with what we found! - AstNode* merged = plan->getAst()->createNodeBinaryOperator( - NODE_TYPE_OPERATOR_BINARY_AND, existingFilter->node(), expr->node()); + AstNode* merged = + plan->getAst()->createNodeBinaryOperator(NODE_TYPE_OPERATOR_BINARY_AND, + existingFilter->node(), + expr->node()); en->setFilter(std::make_unique(plan.get(), plan->getAst(), merged)); } else { @@ -7259,15 +7214,16 @@ void arangodb::aql::moveFiltersIntoEnumerateRule(Optimizer* opt, std::unique_ptr } /// @brief parallelize coordinator GatherNodes -void arangodb::aql::parallelizeGatherRule(Optimizer* opt, std::unique_ptr plan, +void arangodb::aql::parallelizeGatherRule(Optimizer* opt, + std::unique_ptr plan, OptimizerRule const& rule) { TRI_ASSERT(ServerState::instance()->isCoordinator()); - + bool modified = false; // find all GatherNodes in the main query, starting from the query's root node // (the node most south when looking at the query execution plan). - // + // // for now, we effectively stop right after the first GatherNode we found, regardless // of whether we can make that node use parallelism or not. // the reason we have to stop here is that if we have multiple query snippets on a @@ -7275,42 +7231,39 @@ void arangodb::aql::parallelizeGatherRule(Optimizer* opt, std::unique_ptr::allocator_type::arena_type a; ::arangodb::containers::SmallVector nodes{a}; plan->findNodesOfType(nodes, EN::GATHER, true); - if (nodes.size() == 1 && - !plan->contains(EN::TRAVERSAL) && - !plan->contains(EN::SHORTEST_PATH) && - !plan->contains(EN::K_SHORTEST_PATHS) && - !plan->contains(EN::DISTRIBUTE) && - !plan->contains(EN::SCATTER)) { + if (nodes.size() == 1 && !plan->contains(EN::TRAVERSAL) && + !plan->contains(EN::SHORTEST_PATH) && !plan->contains(EN::K_SHORTEST_PATHS) && + !plan->contains(EN::DISTRIBUTE) && !plan->contains(EN::SCATTER)) { GatherNode* gn = ExecutionNode::castTo(nodes[0]); if (!gn->isInSubquery() && gn->isParallelizable()) { @@ -7383,7 +7336,8 @@ void findSubqueriesSuitableForSplicing(ExecutionPlan const& plan, using ResultVector = decltype(result); using BoolVec = std::vector>; - using SuitableNodeSet = std::set, short_alloc>; + using SuitableNodeSet = + std::set, short_alloc>; // This finder adds all subquery nodes in pre-order to its `result` parameter, // and all nodes that are suitable for splicing to `suitableNodes`. Suitable @@ -7474,11 +7428,10 @@ void findSubqueriesSuitableForSplicing(ExecutionPlan const& plan, auto finder = Finder{result, suitableNodes}; plan.root()->walkSubqueriesFirst(finder); - - { // remove unsuitable nodes from result + { // remove unsuitable nodes from result auto i = size_t{0}; auto j = size_t{0}; - for(; j < result.size(); ++j) { + for (; j < result.size(); ++j) { TRI_ASSERT(i <= j); if (suitableNodes.count(result[j]) > 0) { if (i != j) { @@ -7496,7 +7449,7 @@ void findSubqueriesSuitableForSplicing(ExecutionPlan const& plan, result.resize(i); } } -} +} // namespace // Splices in subqueries by replacing subquery nodes by // a SubqueryStartNode and a SubqueryEndNode with the subquery's nodes @@ -7533,10 +7486,11 @@ void arangodb::aql::spliceSubqueriesRule(Optimizer* opt, std::unique_ptrcreateNode(plan.get(), plan->nextId(), sq->outVariable()); + auto start = plan->createNode(plan.get(), plan->nextId(), + sq->outVariable()); // start and end inherit this property from the subquery node start->setIsInSplicedSubquery(sq->isInSplicedSubquery()); @@ -7595,7 +7549,7 @@ void arangodb::aql::spliceSubqueriesRule(Optimizer* opt, std::unique_ptrgetSubquery(); Variable const* inVariable = nullptr; @@ -7613,7 +7567,7 @@ void arangodb::aql::spliceSubqueriesRule(Optimizer* opt, std::unique_ptrcreateNode(plan.get(), plan->nextId(), - inVariable, sq->outVariable()); + inVariable, sq->outVariable()); // start and end inherit this property from the subquery node end->setIsInSplicedSubquery(sq->isInSplicedSubquery()); // insert a SubqueryEndNode after the SubqueryNode sq diff --git a/arangod/Aql/OptimizerRules.h b/arangod/Aql/OptimizerRules.h index da0b5e6b5dc4..af8a8ecb23a9 100644 --- a/arangod/Aql/OptimizerRules.h +++ b/arangod/Aql/OptimizerRules.h @@ -25,8 +25,12 @@ #ifndef ARANGOD_AQL_OPTIMIZER_RULES_H #define ARANGOD_AQL_OPTIMIZER_RULES_H 1 +#include "Aql/ExecutionPlan.h" #include "Aql/OptimizerRulesFeature.h" #include "Basics/Common.h" +#include "ClusterNodes.h" +#include "ExecutionNode.h" +#include "VocBase/vocbase.h" namespace arangodb { namespace aql { @@ -150,6 +154,10 @@ ExecutionNode* distributeInClusterRuleSmartEdgeCollection(ExecutionPlan*, Subque ExecutionNode* originalParent, bool& wasModified); +/// @brief remove scatter/gather and remote nodes for satellite collections +void scatterSatelliteGraphRule(Optimizer*, std::unique_ptr, + OptimizerRule const&); + /// @brief remove scatter/gather and remote nodes for satellite collections void removeSatelliteJoinsRule(Optimizer*, std::unique_ptr, OptimizerRule const&); @@ -268,6 +276,73 @@ void parallelizeGatherRule(Optimizer*, std::unique_ptr, Optimizer //// @brief splice in subqueries void spliceSubqueriesRule(Optimizer*, std::unique_ptr, OptimizerRule const&); +// TODO: to be renamed +// TODO: check return type/method +// TODO: Check inline +inline std::unique_ptr scatterHelperFunction( + std::unique_ptr plan, TRI_vocbase_t* vocbase, + ExecutionNode* node, SortElementVector& elements, size_t numberOfShards, + std::unordered_map& subqueries) { + // insert a scatter node + auto* scatterNode = + new ScatterNode(plan.get(), plan->nextId(), ScatterNode::ScatterType::SHARD); + plan->registerNode(scatterNode); + TRI_ASSERT(node->getDependencies().empty()); + scatterNode->addDependency(node->getDependencies()[0]); + + // insert a remote node + ExecutionNode* remoteNode = + new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); + plan->registerNode(remoteNode); + TRI_ASSERT(scatterNode); + remoteNode->addDependency(scatterNode); + + // re-link with the remote node + node->addDependency(remoteNode); + + // insert another remote node + remoteNode = new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); + plan->registerNode(remoteNode); + TRI_ASSERT(node); + remoteNode->addDependency(node); + + // insert a gather node + auto const sortMode = GatherNode::evaluateSortMode(numberOfShards); + // single-sharded collections don't require any parallelism. collections with more than + // one shard are eligible for later parallelization (the Undefined allows this) + auto const parallelism = (numberOfShards <= 1 ? GatherNode::Parallelism::Serial + : GatherNode::Parallelism::Undefined); + auto* gatherNode = new GatherNode(plan.get(), plan->nextId(), sortMode, parallelism); + plan->registerNode(gatherNode); + TRI_ASSERT(remoteNode); + gatherNode->addDependency(remoteNode); + // On SmartEdge collections we have 0 shards and we need the elements + // to be injected here as well. So do not replace it with > 1 + if (!elements.empty() && numberOfShards != 1) { + gatherNode->elements(elements); + } + + auto const& parents = node->getParents(); + // and now link the gather node with the rest of the plan + if (parents.size() == 1) { + parents[0]->replaceDependency(node->getDependencies()[0], gatherNode); + } + + // check if the node that we modified was at the end of a subquery + auto it = subqueries.find(node); + + if (it != subqueries.end()) { + ExecutionNode::castTo((*it).second)->setSubquery(gatherNode, true); + } + + if (plan->isRoot(node)) { + // if we replaced the root node, set a new root node + plan->root(gatherNode); + } + + return plan; +} + } // namespace aql } // namespace arangodb diff --git a/arangod/Aql/OptimizerRulesFeature.cpp b/arangod/Aql/OptimizerRulesFeature.cpp index e0f289a3ce20..db22a1e396c1 100644 --- a/arangod/Aql/OptimizerRulesFeature.cpp +++ b/arangod/Aql/OptimizerRulesFeature.cpp @@ -376,6 +376,11 @@ void OptimizerRulesFeature::addRules() { OptimizerRule::Flags::ClusterOnly)); #ifdef USE_ENTERPRISE + registerRule("scatter-satellite-graphs", scatterSatelliteGraphRule, + OptimizerRule::scatterSatelliteGraphRule, + OptimizerRule::makeFlags(OptimizerRule::Flags::CanBeDisabled, + OptimizerRule::Flags::ClusterOnly)); + registerRule("remove-satellite-joins", removeSatelliteJoinsRule, OptimizerRule::removeSatelliteJoinsRule, OptimizerRule::makeFlags(OptimizerRule::Flags::CanBeDisabled, From 116662d2f026690176ef2c60cc1e3c4cd557a28d Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 18:42:33 +0100 Subject: [PATCH 05/47] protected to private, reformat --- arangod/Aql/GraphNode.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 8aa10bf07d7b..cb5f4a313263 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -138,10 +138,14 @@ class GraphNode : public ExecutionNode { void injectVertexCollection(aql::Collection const* other); std::vector const collections() const; - void setCollectionToShard(std::map const& map) { _collectionToShard = map; } - void addCollectionToShard(std::string const& coll, std::string const& shard) { _collectionToShard.emplace(coll,shard); } + void setCollectionToShard(std::map const& map) { + _collectionToShard = map; + } + void addCollectionToShard(std::string const& coll, std::string const& shard) { + _collectionToShard.emplace(coll, shard); + } - protected: + public: graph::Graph const* graph() const noexcept; private: From 6c24691a9f1b2e568c103c5c9eba89abe8d19716 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 18:43:05 +0100 Subject: [PATCH 06/47] moved optimizer rule logic out of header file --- arangod/Aql/OptimizerRules.cpp | 63 ++++++++++++++++++++++++++++++- arangod/Aql/OptimizerRules.h | 69 ++-------------------------------- 2 files changed, 65 insertions(+), 67 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 79ef76af4f82..6b4e3b9c7442 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -3614,6 +3614,67 @@ void arangodb::aql::interchangeAdjacentEnumerationsRule(Optimizer* opt, opt->addPlan(std::move(plan), rule, false); } +static void arangodb::aql::createScatterGatherSnippet( + ExecutionPlan& plan, TRI_vocbase_t* vocbase, + ExecutionNode* node, SortElementVector& elements, size_t numberOfShards, + std::unordered_map& subqueries) { + // insert a scatter node + auto* scatterNode = + new ScatterNode(&plan, plan.nextId(), ScatterNode::ScatterType::SHARD); + plan.registerNode(scatterNode); + TRI_ASSERT(node->getDependencies().empty()); + scatterNode->addDependency(node->getDependencies()[0]); + + // insert a remote node + ExecutionNode* remoteNode = + new RemoteNode(&plan, plan.nextId(), vocbase, "", "", ""); + plan.registerNode(remoteNode); + TRI_ASSERT(scatterNode); + remoteNode->addDependency(scatterNode); + + // re-link with the remote node + node->addDependency(remoteNode); + + // insert another remote node + remoteNode = new RemoteNode(&plan, plan.nextId(), vocbase, "", "", ""); + plan.registerNode(remoteNode); + TRI_ASSERT(node); + remoteNode->addDependency(node); + + // insert a gather node + auto const sortMode = GatherNode::evaluateSortMode(numberOfShards); + // single-sharded collections don't require any parallelism. collections with more than + // one shard are eligible for later parallelization (the Undefined allows this) + auto const parallelism = (numberOfShards <= 1 ? GatherNode::Parallelism::Serial + : GatherNode::Parallelism::Undefined); + auto* gatherNode = new GatherNode(&plan, plan.nextId(), sortMode, parallelism); + plan.registerNode(gatherNode); + TRI_ASSERT(remoteNode); + gatherNode->addDependency(remoteNode); + // On SmartEdge collections we have 0 shards and we need the elements + // to be injected here as well. So do not replace it with > 1 + if (!elements.empty() && numberOfShards != 1) { + gatherNode->elements(elements); + } + + auto const& parents = node->getParents(); + // and now link the gather node with the rest of the plan + if (parents.size() == 1) { + parents[0]->replaceDependency(node->getDependencies()[0], gatherNode); + } + + // check if the node that we modified was at the end of a subquery + auto it = subqueries.find(node); + + if (it != subqueries.end()) { + ExecutionNode::castTo((*it).second)->setSubquery(gatherNode, true); + } + + if (plan.isRoot(node)) { + // if we replaced the root node, set a new root node + plan.root(gatherNode); + } +} /// @brief scatter operations in cluster /// this rule inserts scatter, gather and remote nodes so operations on sharded @@ -3721,7 +3782,7 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrnumberOfShards(); - plan = scatterHelperFunction(std::move(plan), vocbase, node, elements, numberOfShards, subqueries); + createScatterGatherSnippet(*plan, vocbase, node, elements, numberOfShards, subqueries); wasModified = true; } diff --git a/arangod/Aql/OptimizerRules.h b/arangod/Aql/OptimizerRules.h index af8a8ecb23a9..d7230855c43b 100644 --- a/arangod/Aql/OptimizerRules.h +++ b/arangod/Aql/OptimizerRules.h @@ -276,72 +276,9 @@ void parallelizeGatherRule(Optimizer*, std::unique_ptr, Optimizer //// @brief splice in subqueries void spliceSubqueriesRule(Optimizer*, std::unique_ptr, OptimizerRule const&); -// TODO: to be renamed -// TODO: check return type/method -// TODO: Check inline -inline std::unique_ptr scatterHelperFunction( - std::unique_ptr plan, TRI_vocbase_t* vocbase, - ExecutionNode* node, SortElementVector& elements, size_t numberOfShards, - std::unordered_map& subqueries) { - // insert a scatter node - auto* scatterNode = - new ScatterNode(plan.get(), plan->nextId(), ScatterNode::ScatterType::SHARD); - plan->registerNode(scatterNode); - TRI_ASSERT(node->getDependencies().empty()); - scatterNode->addDependency(node->getDependencies()[0]); - - // insert a remote node - ExecutionNode* remoteNode = - new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); - plan->registerNode(remoteNode); - TRI_ASSERT(scatterNode); - remoteNode->addDependency(scatterNode); - - // re-link with the remote node - node->addDependency(remoteNode); - - // insert another remote node - remoteNode = new RemoteNode(plan.get(), plan->nextId(), vocbase, "", "", ""); - plan->registerNode(remoteNode); - TRI_ASSERT(node); - remoteNode->addDependency(node); - - // insert a gather node - auto const sortMode = GatherNode::evaluateSortMode(numberOfShards); - // single-sharded collections don't require any parallelism. collections with more than - // one shard are eligible for later parallelization (the Undefined allows this) - auto const parallelism = (numberOfShards <= 1 ? GatherNode::Parallelism::Serial - : GatherNode::Parallelism::Undefined); - auto* gatherNode = new GatherNode(plan.get(), plan->nextId(), sortMode, parallelism); - plan->registerNode(gatherNode); - TRI_ASSERT(remoteNode); - gatherNode->addDependency(remoteNode); - // On SmartEdge collections we have 0 shards and we need the elements - // to be injected here as well. So do not replace it with > 1 - if (!elements.empty() && numberOfShards != 1) { - gatherNode->elements(elements); - } - - auto const& parents = node->getParents(); - // and now link the gather node with the rest of the plan - if (parents.size() == 1) { - parents[0]->replaceDependency(node->getDependencies()[0], gatherNode); - } - - // check if the node that we modified was at the end of a subquery - auto it = subqueries.find(node); - - if (it != subqueries.end()) { - ExecutionNode::castTo((*it).second)->setSubquery(gatherNode, true); - } - - if (plan->isRoot(node)) { - // if we replaced the root node, set a new root node - plan->root(gatherNode); - } - - return plan; -} +void createScatterGatherSnippet(ExecutionPlan& plan, TRI_vocbase_t* vocbase, ExecutionNode* node, + SortElementVector& elements, size_t numberOfShards, + std::unordered_map& subqueries); } // namespace aql } // namespace arangodb From 35e222e7d724e6093503d3105eafd42dd0ce4db3 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 18:43:43 +0100 Subject: [PATCH 07/47] added isEligibleAsSatelliteTraversal method to traversal node --- arangod/Aql/TraversalNode.cpp | 51 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index 5187d984f4f0..d55f003727de 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -42,6 +42,7 @@ #include "Basics/StringUtils.h" #include "Basics/tryEmplaceHelper.h" #include "Cluster/ClusterTraverser.h" +#include "Graph/Graph.h" #ifdef USE_ENTERPRISE #include "Enterprise/Cluster/SmartGraphTraverser.h" #endif @@ -284,7 +285,7 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co for (auto const& cond : VPackObjectIterator(list)) { std::string key = cond.key.copyString(); _vertexConditions.try_emplace(StringUtils::uint64(key), - new AstNode(plan->getAst(), cond.value)); + new AstNode(plan->getAst(), cond.value)); } } @@ -474,7 +475,7 @@ std::unique_ptr TraversalNode::createBlock( TRI_ASSERT(it->second.registerId < RegisterPlan::MaxRegisterId); outputRegisters->emplace(it->second.registerId); outputRegisterMapping.try_emplace(TraversalExecutorInfos::OutputName::VERTEX, - it->second.registerId); + it->second.registerId); } if (usesEdgeOutVariable()) { auto it = varInfo.find(edgeOutVariable()->id); @@ -482,7 +483,7 @@ std::unique_ptr TraversalNode::createBlock( TRI_ASSERT(it->second.registerId < RegisterPlan::MaxRegisterId); outputRegisters->emplace(it->second.registerId); outputRegisterMapping.try_emplace(TraversalExecutorInfos::OutputName::EDGE, - it->second.registerId); + it->second.registerId); } if (usesPathOutVariable()) { auto it = varInfo.find(pathOutVariable()->id); @@ -490,7 +491,7 @@ std::unique_ptr TraversalNode::createBlock( TRI_ASSERT(it->second.registerId < RegisterPlan::MaxRegisterId); outputRegisters->emplace(it->second.registerId); outputRegisterMapping.try_emplace(TraversalExecutorInfos::OutputName::PATH, - it->second.registerId); + it->second.registerId); } auto opts = static_cast(options()); std::unique_ptr traverser; @@ -715,12 +716,9 @@ void TraversalNode::prepareOptions() { for (auto const& jt : _globalVertexConditions) { it.second->addMember(jt); } - opts->_vertexExpressions.try_emplace( - it.first, - arangodb::lazyConstruct([&]{ - return new Expression(_plan, ast, it.second); - }) - ); + opts->_vertexExpressions.try_emplace(it.first, arangodb::lazyConstruct([&] { + return new Expression(_plan, ast, it.second); + })); } if (!_globalVertexConditions.empty()) { auto cond = _plan->getAst()->createNodeNaryOperator(NODE_TYPE_OPERATOR_NARY_AND); @@ -761,26 +759,23 @@ void TraversalNode::registerCondition(bool isConditionOnEdge, uint64_t condition AstNode const* condition) { Ast::getReferencedVariables(condition, _conditionVariables); if (isConditionOnEdge) { - auto[it, emplaced] = _edgeConditions.try_emplace( - conditionLevel, - arangodb::lazyConstruct([&]()-> std::unique_ptr { - auto builder = std::make_unique(this); - builder->addConditionPart(condition); - return builder; - }) - ); + auto [it, emplaced] = _edgeConditions.try_emplace( + conditionLevel, arangodb::lazyConstruct([&]() -> std::unique_ptr { + auto builder = std::make_unique(this); + builder->addConditionPart(condition); + return builder; + })); if (!emplaced) { it->second->addConditionPart(condition); } } else { - auto [it, emplaced] = _vertexConditions.try_emplace( - conditionLevel, - arangodb::lazyConstruct([&]{ - auto cond = _plan->getAst()->createNodeNaryOperator(NODE_TYPE_OPERATOR_NARY_AND); - cond->addMember(condition); - return cond; - }) - ); + auto [it, emplaced] = + _vertexConditions.try_emplace(conditionLevel, arangodb::lazyConstruct([&] { + auto cond = _plan->getAst()->createNodeNaryOperator( + NODE_TYPE_OPERATOR_NARY_AND); + cond->addMember(condition); + return cond; + })); if (!emplaced) { it->second->addMember(condition); } @@ -825,4 +820,8 @@ void TraversalNode::checkConditionsDefined() const { TRI_ASSERT(_toCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); } +bool TraversalNode::isEligibleAsSatelliteTraversal() { + return graph() != nullptr && graph()->isSatellite(); +} + #endif From c604333ec1b2dacb1bc13578bd209315f28aac5f Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 18:44:20 +0100 Subject: [PATCH 08/47] added isEligibleAsSatelliteTraversal method to traversal node --- arangod/Aql/TraversalNode.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index 70b4943c6aca..7454aad69a11 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -186,6 +186,8 @@ class TraversalNode : public GraphNode { bool allDirectionsEqual() const; + bool isEligibleAsSatelliteTraversal() const; + void getConditionVariables(std::vector&) const override; void getPruneVariables(std::vector&) const; From f80b013843248436e0a4a83dbcadc273697ea622 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 28 Jan 2020 18:44:52 +0100 Subject: [PATCH 09/47] added method ensureSatelliteCollectionSharding --- arangod/Graph/GraphManager.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arangod/Graph/GraphManager.h b/arangod/Graph/GraphManager.h index 18ae47ca1d13..2adc07a23fa2 100644 --- a/arangod/Graph/GraphManager.h +++ b/arangod/Graph/GraphManager.h @@ -187,6 +187,8 @@ class GraphManager { #ifdef USE_ENTERPRISE Result ensureSmartCollectionSharding(Graph const* graph, bool waitForSync, std::unordered_set& documentCollections) const; + Result ensureSatelliteCollectionSharding(Graph const* graph, bool waitForSync, + std::unordered_set& documentCollections) const; #endif /** From 16cfd745e8301b5252cf0f582d31e31f635d0fa3 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 13:25:49 +0100 Subject: [PATCH 10/47] preparation for satellites --- arangod/Aql/GraphNode.cpp | 1 + arangod/Aql/OptimizerRules.cpp | 2 +- arangod/Aql/Query.cpp | 6 ++++++ arangod/Aql/SatelliteTraversalNode.h | 1 + arangod/Aql/TraversalNode.cpp | 2 +- arangod/Graph/GraphManager.cpp | 2 +- arangod/Graph/GraphManager.h | 2 ++ 7 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index 9b8e782ec40d..4dea268446cd 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -291,6 +291,7 @@ GraphNode::GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& bas if (base.hasKey("graph") && (base.get("graph").isString())) { graphName = base.get("graph").copyString(); if (base.hasKey("graphDefinition")) { + // load graph and store pointer _graphObj = plan->getAst()->query()->lookupGraphByName(graphName); if (_graphObj == nullptr) { diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 6b4e3b9c7442..db4d20ced230 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -3614,7 +3614,7 @@ void arangodb::aql::interchangeAdjacentEnumerationsRule(Optimizer* opt, opt->addPlan(std::move(plan), rule, false); } -static void arangodb::aql::createScatterGatherSnippet( +void arangodb::aql::createScatterGatherSnippet( ExecutionPlan& plan, TRI_vocbase_t* vocbase, ExecutionNode* node, SortElementVector& elements, size_t numberOfShards, std::unordered_map& subqueries) { diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 6f9fb6d60c61..9b55a896f934 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -1523,7 +1523,13 @@ graph::Graph const* Query::lookupGraphByName(std::string const& name) { } graph::GraphManager graphManager{_vocbase, _contextOwnedByExterior}; +#ifdef USE_ENTERPRISE + // TODO: create with proper constructor + auto g = graphManager.lookupGraphByName(name); +#else auto g = graphManager.lookupGraphByName(name); +#endif + if (g.fail()) { return nullptr; diff --git a/arangod/Aql/SatelliteTraversalNode.h b/arangod/Aql/SatelliteTraversalNode.h index 7a28ebb45c77..0bcf47bb273f 100644 --- a/arangod/Aql/SatelliteTraversalNode.h +++ b/arangod/Aql/SatelliteTraversalNode.h @@ -29,6 +29,7 @@ namespace arangodb::aql { class SatelliteTraversalNode : public TraversalNode, public CollectionAccessingNode { + public: SatelliteTraversalNode(TraversalNode&& traversalNode, aql::Collection const& collection); }; diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index d55f003727de..59b3848ebfb0 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -820,7 +820,7 @@ void TraversalNode::checkConditionsDefined() const { TRI_ASSERT(_toCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); } -bool TraversalNode::isEligibleAsSatelliteTraversal() { +bool TraversalNode::isEligibleAsSatelliteTraversal() const { return graph() != nullptr && graph()->isSatellite(); } diff --git a/arangod/Graph/GraphManager.cpp b/arangod/Graph/GraphManager.cpp index 4dcfdc57979c..2abed4651827 100644 --- a/arangod/Graph/GraphManager.cpp +++ b/arangod/Graph/GraphManager.cpp @@ -563,7 +563,7 @@ Result GraphManager::ensureCollections(Graph const* graph, bool waitForSync) con #ifdef USE_ENTERPRISE { - Result res = ensureSmartCollectionSharding(graph, waitForSync, documentCollectionsToCreate); + Result res = ensureEnterpriseCollectionSharding(graph, waitForSync, documentCollectionsToCreate); if (res.fail()) { return res; } diff --git a/arangod/Graph/GraphManager.h b/arangod/Graph/GraphManager.h index 2adc07a23fa2..4aef313acbce 100644 --- a/arangod/Graph/GraphManager.h +++ b/arangod/Graph/GraphManager.h @@ -185,6 +185,8 @@ class GraphManager { private: #ifdef USE_ENTERPRISE + Result ensureEnterpriseCollectionSharding(Graph const* graph, bool waitForSync, + std::unordered_set& documentCollections) const; Result ensureSmartCollectionSharding(Graph const* graph, bool waitForSync, std::unordered_set& documentCollections) const; Result ensureSatelliteCollectionSharding(Graph const* graph, bool waitForSync, From cfd855c06909abd0f67db3389f32f3c3c0ee5029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 29 Jan 2020 13:41:14 +0100 Subject: [PATCH 11/47] Replaced the SatelliteTraversalNode move constructor with a copy constructor --- arangod/Aql/ExecutionNode.cpp | 61 +++++++++++++++----------- arangod/Aql/ExecutionNode.h | 13 +++++- arangod/Aql/GraphNode.cpp | 28 ++++++++++-- arangod/Aql/GraphNode.h | 10 +++++ arangod/Aql/RegisterPlan.h | 7 ++- arangod/Aql/SatelliteTraversalNode.cpp | 23 +++++----- arangod/Aql/SatelliteTraversalNode.h | 7 ++- arangod/Aql/TraversalNode.cpp | 61 ++++++++------------------ arangod/Aql/TraversalNode.h | 12 +++-- 9 files changed, 130 insertions(+), 92 deletions(-) diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index 5e491ef8eddc..4e016201cb20 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/arangod/Aql/ExecutionNode.cpp @@ -463,6 +463,12 @@ ExecutionNode::ExecutionNode(ExecutionPlan* plan, VPackSlice const& slice) _isInSplicedSubquery = VelocyPackHelper::getBooleanValue(slice, "isInSplicedSubquery", false); } +ExecutionNode::ExecutionNode(ExecutionPlan& plan, ExecutionNode const& other) + : ExecutionNode(&plan, other.id()) { + TRI_ASSERT(&plan == other.plan()); + other.cloneWithoutRegisteringAndDependencies(plan, *this, false); +} + /// @brief toVelocyPack, export an ExecutionNode to VelocyPack void ExecutionNode::toVelocyPack(VPackBuilder& builder, unsigned flags, bool keepTopLevelOpen) const { @@ -485,59 +491,64 @@ ExecutionNode* ExecutionNode::cloneHelper(std::unique_ptr other, bool withDependencies, bool withProperties) const { ExecutionPlan* plan = other->plan(); - if (plan == _plan) { + cloneWithoutRegisteringAndDependencies(*plan, *other, withProperties); + + auto* registeredNode = plan->registerNode(std::move(other)); + + if (withDependencies) { + cloneDependencies(plan, registeredNode, withProperties); + } + + return registeredNode; +} + +void ExecutionNode::cloneWithoutRegisteringAndDependencies(ExecutionPlan& plan, ExecutionNode& other, bool withProperties) const { + + if (&plan == _plan) { // same execution plan for source and target // now assign a new id to the cloned node, otherwise it will fail // upon node registration and/or its meaning is ambiguous - other->setId(plan->nextId()); + other.setId(plan.nextId()); // cloning with properties will only work if we clone a node into // a different plan TRI_ASSERT(!withProperties); } - other->_regsToClear = _regsToClear; - other->_depth = _depth; - other->_varUsageValid = _varUsageValid; - other->_isInSplicedSubquery = _isInSplicedSubquery; + other._regsToClear = _regsToClear; + other._depth = _depth; + other._varUsageValid = _varUsageValid; + other._isInSplicedSubquery = _isInSplicedSubquery; if (withProperties) { - auto allVars = plan->getAst()->variables(); + auto allVars = plan.getAst()->variables(); // Create new structures on the new AST... - other->_varsUsedLater.reserve(_varsUsedLater.size()); + other._varsUsedLater.reserve(_varsUsedLater.size()); for (auto const& orgVar : _varsUsedLater) { auto var = allVars->getVariable(orgVar->id); TRI_ASSERT(var != nullptr); - other->_varsUsedLater.insert(var); + other._varsUsedLater.insert(var); } - other->_varsValid.reserve(_varsValid.size()); + other._varsValid.reserve(_varsValid.size()); for (auto const& orgVar : _varsValid) { auto var = allVars->getVariable(orgVar->id); TRI_ASSERT(var != nullptr); - other->_varsValid.insert(var); + other._varsValid.insert(var); } if (_registerPlan.get() != nullptr) { auto otherRegisterPlan = - std::shared_ptr(_registerPlan->clone(plan, _plan)); - other->_registerPlan = otherRegisterPlan; + std::shared_ptr(_registerPlan->clone(&plan, _plan)); + other._registerPlan = otherRegisterPlan; } } else { // point to current AST -> don't do deep copies. - other->_varsUsedLater = _varsUsedLater; - other->_varsValid = _varsValid; - other->_registerPlan = _registerPlan; + other._varsUsedLater = _varsUsedLater; + other._varsValid = _varsValid; + other._registerPlan = _registerPlan; } - - auto* registeredNode = plan->registerNode(std::move(other)); - - if (withDependencies) { - cloneDependencies(plan, registeredNode, withProperties); - } - - return registeredNode; } /// @brief helper for cloning, use virtual clone methods for dependencies @@ -1068,8 +1079,6 @@ RegisterId ExecutionNode::getNrOutputRegisters() const { ExecutionNode::ExecutionNode(ExecutionPlan* plan, size_t id) : _id(id), _depth(0), _varUsageValid(false), _plan(plan), _isInSplicedSubquery(false) {} -ExecutionNode::~ExecutionNode() = default; - size_t ExecutionNode::id() const { return _id; } void ExecutionNode::swapFirstDependency(ExecutionNode* node) { diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h index ce1035a76d0c..565c926c11d7 100644 --- a/arangod/Aql/ExecutionNode.h +++ b/arangod/Aql/ExecutionNode.h @@ -162,6 +162,14 @@ class ExecutionNode { ExecutionNode() = delete; ExecutionNode(ExecutionNode const&) = delete; ExecutionNode& operator=(ExecutionNode const&) = delete; + protected: + /// @brief Clone constructor, used for constructors of derived classes. + /// Does not clone recursively, does not clone properties (`other.plan()` is + /// expected to be the same as `plan)`, and does not register this node in the + /// plan. + ExecutionNode(ExecutionPlan& plan, ExecutionNode const& other); + + public: /// @brief constructor using an id ExecutionNode(ExecutionPlan* plan, size_t id); @@ -170,7 +178,7 @@ class ExecutionNode { ExecutionNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& slice); /// @brief destructor, free dependencies - virtual ~ExecutionNode(); + virtual ~ExecutionNode() = default; public: /// @brief factory from JSON @@ -303,6 +311,9 @@ class ExecutionNode { ExecutionNode* cloneHelper(std::unique_ptr Other, bool withDependencies, bool withProperties) const; + void cloneWithoutRegisteringAndDependencies(ExecutionPlan& plan, ExecutionNode& other, + bool withProperties) const; + /// @brief helper for cloning, use virtual clone methods for dependencies void cloneDependencies(ExecutionPlan* plan, ExecutionNode* theClone, bool withProperties) const; diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index 4dea268446cd..ff25e1037934 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -401,12 +401,17 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, _options(std::move(options)), _optionsBuilt(false), _isSmart(false) { + setGraphInfoAndCopyColls(edgeColls, vertexColls); +} + +void GraphNode::setGraphInfoAndCopyColls(std::vector> const& edgeColls, + std::vector> const& vertexColls) { _graphInfo.openArray(); for (auto& it : edgeColls) { // Collections cannot be copied. So we need to create new ones to prevent // leaks - _edgeColls.emplace_back(std::make_unique(it->name(), _vocbase, - AccessMode::Type::READ)); + _edgeColls.emplace_back(std::make_unique(it->name(), _vocbase, + AccessMode::Type::READ)); _graphInfo.add(VPackValue(it->name())); } _graphInfo.close(); @@ -415,10 +420,27 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, // Collections cannot be copied. So we need to create new ones to prevent // leaks _vertexColls.emplace_back( - std::make_unique(it->name(), _vocbase, AccessMode::Type::READ)); + std::make_unique(it->name(), _vocbase, AccessMode::Type::READ)); } } +GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, + std::unique_ptr options) + : ExecutionNode(plan, other), + _vocbase(other._vocbase), + _vertexOutVariable(nullptr), + _edgeOutVariable(nullptr), + _graphObj(nullptr), + _tmpObjVariable(_plan->getAst()->variables()->createTemporaryVariable()), + _tmpObjVarNode(_plan->getAst()->createNodeReference(_tmpObjVariable)), + _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), + _directions(other._directions), + _options(std::move(options)), + _optionsBuilt(false), + _isSmart(false) { + setGraphInfoAndCopyColls(other.edgeColls(), other.vertexColls()); +} + std::string const& GraphNode::collectionToShardName(std::string const& collName) const { if (_collectionToShard.empty()) { return collName; diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index cb5f4a313263..94711afc6e3f 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -67,6 +67,13 @@ class GraphNode : public ExecutionNode { std::vector directions, std::unique_ptr options); + /// @brief Clone constructor, used for constructors of derived classes. + /// Does not clone recursively, does not clone properties (`other.plan()` is + /// expected to be the same as `plan)`, and does not register this node in the + /// plan. + GraphNode(ExecutionPlan& plan, GraphNode const& other, + std::unique_ptr options); + std::string const& collectionToShardName(std::string const& collName) const; public: @@ -151,6 +158,9 @@ class GraphNode : public ExecutionNode { private: void addEdgeCollection(std::string const& n, TRI_edge_direction_e dir); + void setGraphInfoAndCopyColls(std::vector> const& edgeColls, + std::vector> const& vertexColls); + protected: /// @brief the database TRI_vocbase_t* _vocbase; diff --git a/arangod/Aql/RegisterPlan.h b/arangod/Aql/RegisterPlan.h index 2e589660a655..0f51d139f63e 100644 --- a/arangod/Aql/RegisterPlan.h +++ b/arangod/Aql/RegisterPlan.h @@ -25,6 +25,7 @@ #ifndef ARANGOD_AQL_REGISTER_PLAN_H #define ARANGOD_AQL_REGISTER_PLAN_H 1 +#include "Aql/ExecutionNode.h" #include "Aql/WalkerWorker.h" #include "Aql/types.h" #include "Basics/Common.h" @@ -32,8 +33,7 @@ #include -namespace arangodb { -namespace aql { +namespace arangodb::aql { class ExecutionNode; class ExecutionPlan; @@ -108,7 +108,6 @@ struct RegisterPlan final : public WalkerWorker { static constexpr RegisterId SUBQUERY_DEPTH_REGISTER = 0; }; -} // namespace aql -} // namespace arangodb +} // namespace arangodb::aql #endif diff --git a/arangod/Aql/SatelliteTraversalNode.cpp b/arangod/Aql/SatelliteTraversalNode.cpp index aef34cf5364a..5bf445e9ca9c 100644 --- a/arangod/Aql/SatelliteTraversalNode.cpp +++ b/arangod/Aql/SatelliteTraversalNode.cpp @@ -22,24 +22,23 @@ #include "SatelliteTraversalNode.h" +#include "Aql/Collection.h" #include "Basics/Exceptions.h" #include "Graph/Graph.h" using namespace arangodb; using namespace arangodb::aql; -SatelliteTraversalNode::SatelliteTraversalNode(TraversalNode&& traversalNode, +SatelliteTraversalNode::SatelliteTraversalNode(ExecutionPlan& plan, + TraversalNode const& traversalNode, aql::Collection const& collection) - : TraversalNode(std::move(traversalNode)), CollectionAccessingNode(&collection) { - // TODO Replace the following checks with a check on traversalNode->isEligibleAsSatelliteTraversal(). - if (graph() == nullptr) { - TRI_ASSERT(false); - THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL_AQL, "Logic error: satellite traversals currently only supported on named graphs"); + : TraversalNode(plan, traversalNode), CollectionAccessingNode(&collection) { + TRI_ASSERT(&plan == traversalNode.plan()); + TRI_ASSERT(collection.vocbase() == traversalNode.vocbase()); + TRI_ASSERT(traversalNode.isEligibleAsSatelliteTraversal()); + if (ADB_UNLIKELY(!traversalNode.isEligibleAsSatelliteTraversal())) { + THROW_ARANGO_EXCEPTION_MESSAGE( + TRI_ERROR_INTERNAL_AQL, + "Logic error: traversal not eligible for satellite traversal"); } - - if (!graph()->isSatellite()) { - TRI_ASSERT(false); - THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL_AQL, "Logic error: satellite traversals on non-satellite graph"); - } - } diff --git a/arangod/Aql/SatelliteTraversalNode.h b/arangod/Aql/SatelliteTraversalNode.h index 0bcf47bb273f..00270961ab40 100644 --- a/arangod/Aql/SatelliteTraversalNode.h +++ b/arangod/Aql/SatelliteTraversalNode.h @@ -30,7 +30,12 @@ namespace arangodb::aql { class SatelliteTraversalNode : public TraversalNode, public CollectionAccessingNode { public: - SatelliteTraversalNode(TraversalNode&& traversalNode, aql::Collection const& collection); + SatelliteTraversalNode(ExecutionPlan& plan, TraversalNode const& traversalNode, + aql::Collection const& collection); + + // Resolve ambiguous overloads + using CollectionAccessingNode::collection; + using CollectionAccessingNode::vocbase; }; } // namespace arangodb::aql diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index 59b3848ebfb0..a7afdae4249d 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -174,47 +174,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocb _fromCondition(nullptr), _toCondition(nullptr) {} -TraversalNode::TraversalNode(TraversalNode&& other) - : GraphNode(other._plan, other._id, other._vocbase, other._edgeColls, other._vertexColls, - std::move(other._directions), std::move(other._options)), - _pathOutVariable(other._pathOutVariable), - _inVariable(other._inVariable), - _vertexId(std::move(other._vertexId)), - _condition(std::move(other._condition)), - _conditionVariables(std::move(other._conditionVariables)), - _fromCondition(other._fromCondition), - _toCondition(other._toCondition), - _pruneExpression(std::move(other._pruneExpression)), - _globalEdgeConditions(std::move(other._globalEdgeConditions)), - _globalVertexConditions(std::move(other._globalVertexConditions)), - _edgeConditions(std::move(other._edgeConditions)), - _vertexConditions(std::move(other._vertexConditions)), - _pruneVariables(std::move(other._pruneVariables)) { - // TODO Maybe we need to copy/move members of ExecutionNode as well! - - // Copy remaining members of GraphNode - _vertexOutVariable = other._vertexOutVariable; - _edgeOutVariable = other._edgeOutVariable; - _graphObj = other._graphObj; - _defaultDirection = other._defaultDirection; - _isSmart = other._isSmart; - - // Clear everything that's not moved, especially everything involving manual - // memory management. - other._plan = nullptr; - other._id = -1; - other._vocbase = nullptr; - other._edgeColls.clear(); - other._vertexColls.clear(); - other._pathOutVariable = nullptr; - other._inVariable = nullptr; - other._fromCondition = nullptr; - other._toCondition = nullptr; - other._vertexOutVariable = nullptr; - other._edgeOutVariable = nullptr; - other._graphObj = nullptr; -} - TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base) : GraphNode(plan, base), _pathOutVariable(nullptr), @@ -314,6 +273,14 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co #endif } +TraversalNode::TraversalNode(ExecutionPlan& plan, TraversalNode const& other) + : GraphNode(plan, other, std::make_unique(*options())), + _pathOutVariable(nullptr), + _inVariable(other._inVariable), + _vertexId(other._vertexId), + _fromCondition(nullptr), + _toCondition(nullptr) {} + TraversalNode::~TraversalNode() = default; int TraversalNode::checkIsOutVariable(size_t variableId) const { @@ -568,7 +535,7 @@ std::unique_ptr TraversalNode::createBlock( ExecutionNode* TraversalNode::clone(ExecutionPlan* plan, bool withDependencies, bool withProperties) const { TRI_ASSERT(!_optionsBuilt); - auto oldOpts = static_cast(options()); + auto oldOpts = options(); std::unique_ptr tmp = std::make_unique(*oldOpts); auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, _vertexColls, _inVariable, _vertexId, @@ -824,4 +791,14 @@ bool TraversalNode::isEligibleAsSatelliteTraversal() const { return graph() != nullptr && graph()->isSatellite(); } +auto TraversalNode::options() const -> TraverserOptions* { +#ifdef ARANGODB_ENABLE_MAINTAINER_MODE + auto* opts = dynamic_cast(GraphNode::options()); + TRI_ASSERT((GraphNode::options() == nullptr) == (opts == nullptr)); +#else + auto* opts = static_cast(GraphNode::options()); +#endif + return opts; +} + #endif diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index 7454aad69a11..1e898757dd3d 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -94,9 +94,11 @@ class TraversalNode : public GraphNode { std::unique_ptr options); protected: - /// @brief Move from another traversal node. Used to create a - /// SatelliteTraversalNode from a TraversalNode, see the constructor there. - TraversalNode(TraversalNode&& other); + /// @brief Clone constructor, used for constructors of derived classes. + /// Does not clone recursively, does not clone properties (`other.plan()` is + /// expected to be the same as `plan)`, and does not register this node in the + /// plan. + TraversalNode(ExecutionPlan& plan, TraversalNode const& other); public: /// @brief return the type of the node @@ -201,6 +203,10 @@ class TraversalNode : public GraphNode { // You are not responsible for it! Expression* pruneExpression() const { return _pruneExpression.get(); } + /// @brief Overrides GraphNode::Options with a more specific return type + /// (casts graph::BaseOptions* into traverser::TraverserOptions*) + auto options() const -> traverser::TraverserOptions*; + private: #ifdef ARANGODB_ENABLE_MAINTAINER_MODE void checkConditionsDefined() const; From ae32adaf5a4f8560b521ff6c1ee2fd828d987d1f Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 13:59:46 +0100 Subject: [PATCH 12/47] format, remove graphmanager ee switch --- arangod/Aql/Query.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 9b55a896f934..89e85ec54cf4 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -912,7 +912,7 @@ QueryResultV8 Query::executeV8(v8::Isolate* isolate, QueryRegistry* registry) { if (useQueryCache) { val.toVelocyPack(_trx.get(), *builder, true); } - + if (V8PlatformFeature::isOutOfMemory(isolate)) { THROW_ARANGO_EXCEPTION(TRI_ERROR_OUT_OF_MEMORY); } @@ -1016,19 +1016,21 @@ ExecutionState Query::finalize(QueryResult& result) { if (_queryOptions.profile >= PROFILE_LEVEL_BLOCKS) { if (ServerState::instance()->isCoordinator()) { std::vector const collectionNodeTypes{ - arangodb::aql::ExecutionNode::ENUMERATE_COLLECTION, - arangodb::aql::ExecutionNode::INDEX, - arangodb::aql::ExecutionNode::REMOVE, arangodb::aql::ExecutionNode::INSERT, - arangodb::aql::ExecutionNode::UPDATE, arangodb::aql::ExecutionNode::REPLACE, - arangodb::aql::ExecutionNode::UPSERT}; + arangodb::aql::ExecutionNode::ENUMERATE_COLLECTION, + arangodb::aql::ExecutionNode::INDEX, + arangodb::aql::ExecutionNode::REMOVE, + arangodb::aql::ExecutionNode::INSERT, + arangodb::aql::ExecutionNode::UPDATE, + arangodb::aql::ExecutionNode::REPLACE, + arangodb::aql::ExecutionNode::UPSERT}; ::arangodb::containers::SmallVector::allocator_type::arena_type a; ::arangodb::containers::SmallVector nodes{a}; _plan->findNodesOfType(nodes, collectionNodeTypes, true); for (auto& n : nodes) { - // clear shards so we get back the full collection name when serializing - // the plan + // clear shards so we get back the full collection name when + // serializing the plan auto cn = dynamic_cast(n); if (cn) { cn->setUsedShard(""); @@ -1522,14 +1524,7 @@ graph::Graph const* Query::lookupGraphByName(std::string const& name) { return it->second.get(); } graph::GraphManager graphManager{_vocbase, _contextOwnedByExterior}; - -#ifdef USE_ENTERPRISE - // TODO: create with proper constructor auto g = graphManager.lookupGraphByName(name); -#else - auto g = graphManager.lookupGraphByName(name); -#endif - if (g.fail()) { return nullptr; From 74df9c2ec882d147630c46c793a18ec90cb9d57a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 29 Jan 2020 14:22:33 +0100 Subject: [PATCH 13/47] Moved SatelliteTraversalNode to the enterprise repo --- arangod/Aql/SatelliteTraversalNode.cpp | 44 -------------------------- arangod/Aql/SatelliteTraversalNode.h | 43 ------------------------- arangod/CMakeLists.txt | 1 - 3 files changed, 88 deletions(-) delete mode 100644 arangod/Aql/SatelliteTraversalNode.cpp delete mode 100644 arangod/Aql/SatelliteTraversalNode.h diff --git a/arangod/Aql/SatelliteTraversalNode.cpp b/arangod/Aql/SatelliteTraversalNode.cpp deleted file mode 100644 index 5bf445e9ca9c..000000000000 --- a/arangod/Aql/SatelliteTraversalNode.cpp +++ /dev/null @@ -1,44 +0,0 @@ -//////////////////////////////////////////////////////////////////////////////// -/// DISCLAIMER -/// -/// Copyright 2020 ArangoDB GmbH, Cologne, Germany -/// -/// Licensed under the Apache License, Version 2.0 (the "License"); -/// you may not use this file except in compliance with the License. -/// You may obtain a copy of the License at -/// -/// http://www.apache.org/licenses/LICENSE-2.0 -/// -/// Unless required by applicable law or agreed to in writing, software -/// distributed under the License is distributed on an "AS IS" BASIS, -/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -/// See the License for the specific language governing permissions and -/// limitations under the License. -/// -/// Copyright holder is ArangoDB GmbH, Cologne, Germany -/// -/// @author Tobias Gödderz -//////////////////////////////////////////////////////////////////////////////// - -#include "SatelliteTraversalNode.h" - -#include "Aql/Collection.h" -#include "Basics/Exceptions.h" -#include "Graph/Graph.h" - -using namespace arangodb; -using namespace arangodb::aql; - -SatelliteTraversalNode::SatelliteTraversalNode(ExecutionPlan& plan, - TraversalNode const& traversalNode, - aql::Collection const& collection) - : TraversalNode(plan, traversalNode), CollectionAccessingNode(&collection) { - TRI_ASSERT(&plan == traversalNode.plan()); - TRI_ASSERT(collection.vocbase() == traversalNode.vocbase()); - TRI_ASSERT(traversalNode.isEligibleAsSatelliteTraversal()); - if (ADB_UNLIKELY(!traversalNode.isEligibleAsSatelliteTraversal())) { - THROW_ARANGO_EXCEPTION_MESSAGE( - TRI_ERROR_INTERNAL_AQL, - "Logic error: traversal not eligible for satellite traversal"); - } -} diff --git a/arangod/Aql/SatelliteTraversalNode.h b/arangod/Aql/SatelliteTraversalNode.h deleted file mode 100644 index 00270961ab40..000000000000 --- a/arangod/Aql/SatelliteTraversalNode.h +++ /dev/null @@ -1,43 +0,0 @@ -//////////////////////////////////////////////////////////////////////////////// -/// DISCLAIMER -/// -/// Copyright 2020 ArangoDB GmbH, Cologne, Germany -/// -/// Licensed under the Apache License, Version 2.0 (the "License"); -/// you may not use this file except in compliance with the License. -/// You may obtain a copy of the License at -/// -/// http://www.apache.org/licenses/LICENSE-2.0 -/// -/// Unless required by applicable law or agreed to in writing, software -/// distributed under the License is distributed on an "AS IS" BASIS, -/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -/// See the License for the specific language governing permissions and -/// limitations under the License. -/// -/// Copyright holder is ArangoDB GmbH, Cologne, Germany -/// -/// @author Tobias Gödderz -//////////////////////////////////////////////////////////////////////////////// - -#ifndef ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H -#define ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H - -#include "Aql/CollectionAccessingNode.h" -#include "Aql/TraversalNode.h" - -namespace arangodb::aql { - -class SatelliteTraversalNode : public TraversalNode, public CollectionAccessingNode { - public: - SatelliteTraversalNode(ExecutionPlan& plan, TraversalNode const& traversalNode, - aql::Collection const& collection); - - // Resolve ambiguous overloads - using CollectionAccessingNode::collection; - using CollectionAccessingNode::vocbase; -}; - -} // namespace arangodb::aql - -#endif // ARANGOD_AQL_SATELLITE_TRAVERSAL_NODE_H diff --git a/arangod/CMakeLists.txt b/arangod/CMakeLists.txt index f8de5497bab4..d9736d0c0f88 100644 --- a/arangod/CMakeLists.txt +++ b/arangod/CMakeLists.txt @@ -329,7 +329,6 @@ set(LIB_ARANGO_AQL_SOURCES Aql/RemoteExecutor.cpp Aql/RestAqlHandler.cpp Aql/ReturnExecutor.cpp - Aql/SatelliteTraversalNode.cpp Aql/ScatterExecutor.cpp Aql/Scopes.cpp Aql/ShadowAqlItemRow.cpp From e8072c405281a7d6682f34efd6f041e6d03dc6af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 29 Jan 2020 15:29:46 +0100 Subject: [PATCH 14/47] Bugfix in scatter rule --- arangod/Aql/OptimizerRules.cpp | 27 ++++++++++++++++----------- arangod/Aql/OptimizerRules.h | 9 ++++++--- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index db4d20ced230..7d324daba6d6 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -3615,15 +3615,18 @@ void arangodb::aql::interchangeAdjacentEnumerationsRule(Optimizer* opt, } void arangodb::aql::createScatterGatherSnippet( - ExecutionPlan& plan, TRI_vocbase_t* vocbase, - ExecutionNode* node, SortElementVector& elements, size_t numberOfShards, - std::unordered_map& subqueries) { + ExecutionPlan& plan, TRI_vocbase_t* vocbase, ExecutionNode* node, + bool const isRootNode, std::vector const& nodeDependencies, + std::vector const& nodeParents, + SortElementVector const& elements, size_t const numberOfShards, + std::unordered_map const& subqueries) { // insert a scatter node auto* scatterNode = new ScatterNode(&plan, plan.nextId(), ScatterNode::ScatterType::SHARD); plan.registerNode(scatterNode); TRI_ASSERT(node->getDependencies().empty()); - scatterNode->addDependency(node->getDependencies()[0]); + TRI_ASSERT(!nodeDependencies.empty()); + scatterNode->addDependency(nodeDependencies[0]); // insert a remote node ExecutionNode* remoteNode = @@ -3657,10 +3660,9 @@ void arangodb::aql::createScatterGatherSnippet( gatherNode->elements(elements); } - auto const& parents = node->getParents(); // and now link the gather node with the rest of the plan - if (parents.size() == 1) { - parents[0]->replaceDependency(node->getDependencies()[0], gatherNode); + if (nodeParents.size() == 1) { + nodeParents[0]->replaceDependency(nodeDependencies[0], gatherNode); } // check if the node that we modified was at the end of a subquery @@ -3670,7 +3672,7 @@ void arangodb::aql::createScatterGatherSnippet( ExecutionNode::castTo((*it).second)->setSubquery(gatherNode, true); } - if (plan.isRoot(node)) { + if (isRootNode) { // if we replaced the root node, set a new root node plan.root(gatherNode); } @@ -3710,8 +3712,9 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrgetParents(); auto const deps = node->getDependencies(); TRI_ASSERT(deps.size() == 1); @@ -3725,6 +3728,7 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrisRoot(node); plan->unlinkNode(node, true); auto const nodeType = node->getType(); @@ -3782,7 +3786,8 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, std::unique_ptrnumberOfShards(); - createScatterGatherSnippet(*plan, vocbase, node, elements, numberOfShards, subqueries); + createScatterGatherSnippet(*plan, vocbase, node, isRootNode, deps, parents, + elements, numberOfShards, subqueries); wasModified = true; } diff --git a/arangod/Aql/OptimizerRules.h b/arangod/Aql/OptimizerRules.h index d7230855c43b..13ea878061e3 100644 --- a/arangod/Aql/OptimizerRules.h +++ b/arangod/Aql/OptimizerRules.h @@ -276,9 +276,12 @@ void parallelizeGatherRule(Optimizer*, std::unique_ptr, Optimizer //// @brief splice in subqueries void spliceSubqueriesRule(Optimizer*, std::unique_ptr, OptimizerRule const&); -void createScatterGatherSnippet(ExecutionPlan& plan, TRI_vocbase_t* vocbase, ExecutionNode* node, - SortElementVector& elements, size_t numberOfShards, - std::unordered_map& subqueries); +void createScatterGatherSnippet(ExecutionPlan& plan, TRI_vocbase_t* vocbase, + ExecutionNode* node, bool isRootNode, + std::vector const& nodeDependencies, + std::vector const& nodeParents, + SortElementVector const& elements, size_t numberOfShards, + std::unordered_map const& subqueries); } // namespace aql } // namespace arangodb From 9761b4d742c7f6dfb2b6be0d3e06688aa855a780 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 16:44:33 +0100 Subject: [PATCH 15/47] added repl factor sat check, needs tidy up later --- arangod/Sharding/ShardingInfo.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arangod/Sharding/ShardingInfo.cpp b/arangod/Sharding/ShardingInfo.cpp index 4091652d579f..6cd6c0d73376 100644 --- a/arangod/Sharding/ShardingInfo.cpp +++ b/arangod/Sharding/ShardingInfo.cpp @@ -516,11 +516,17 @@ Result ShardingInfo::validateShardsAndReplicationFactor(arangodb::velocypack::Sl if (enforceReplicationFactor) { auto enforceSlice = slice.get("enforceReplicationFactor"); - if (!enforceSlice.isBool() || enforceSlice.getBool()) { + if (!enforceSlice.isBool() || enforceSlice.getBool()) { auto replicationFactorSlice = slice.get(StaticStrings::ReplicationFactor); if (replicationFactorSlice.isNumber()) { int64_t replicationFactorProbe = replicationFactorSlice.getNumber(); - if (replicationFactorProbe <= 0) { + if (replicationFactorProbe == 0) { + // TODO: Which configuration for satellites are valid regarding minRepl and writeConcern + LOG_DEVEL << "will create satellite collection - remove me after cleanup"; + // valid for creating a satellite collection + return Result(); + } + if (replicationFactorProbe < 0) { return Result(TRI_ERROR_BAD_PARAMETER, "invalid value for replicationFactor"); } From f082be839da7ac3b8be568267b9a433f3acda607 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 19:05:51 +0100 Subject: [PATCH 16/47] added indicator to satellite graph tile --- .../APP/frontend/js/templates/graphManagementView.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html index d33c8a4a56da..88b2f545d8d7 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html @@ -49,6 +49,7 @@ <% graphs.forEach(function(graph) { var graphName = graph.get("_key"); var isSmart = graph.get("isSmart"); + var isSatellite = (graph.get("replicationFactor") == "satellite"); %>
@@ -62,6 +63,8 @@
<% if (isSmart === true) { %>
Smart
 + <% } else if (isSatellite === true) { %> +
Satellite
 <% } %>
<%=graphName %>
From f939bf3526d0ac54f419121fbfabb7d8a09b5487 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 19:54:27 +0100 Subject: [PATCH 17/47] added modal entry point for satellite graphs - ui --- .../aardvark/APP/frontend/js/templates/modalGraphTable.html | 1 + 1 file changed, 1 insertion(+) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/modalGraphTable.html b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/modalGraphTable.html index e4c204c0660e..2eb643dc0d7e 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/modalGraphTable.html +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/modalGraphTable.html @@ -2,6 +2,7 @@ From ece8b72185661fc41c7c5a577e555178a89c0e67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 29 Jan 2020 20:02:33 +0100 Subject: [PATCH 18/47] Several bugfixes --- ...EngineInfoContainerDBServerServerBased.cpp | 4 +- arangod/Aql/ExecutionPlan.cpp | 23 +++++++ arangod/Aql/ExecutionPlan.h | 2 + arangod/Aql/GraphNode.cpp | 3 + arangod/Aql/GraphNode.h | 2 +- arangod/Aql/KShortestPathsNode.cpp | 7 +- arangod/Aql/KShortestPathsNode.h | 1 + arangod/Aql/OptimizerRule.h | 1 - arangod/Aql/OptimizerRules.cpp | 36 ---------- arangod/Aql/OptimizerRules.h | 4 -- arangod/Aql/OptimizerRulesFeature.cpp | 4 -- arangod/Aql/Query.cpp | 3 + arangod/Aql/ShortestPathNode.cpp | 7 +- arangod/Aql/ShortestPathNode.h | 1 + arangod/Aql/TraversalNode.cpp | 69 +++++++++++-------- arangod/Aql/TraversalNode.h | 7 +- 16 files changed, 92 insertions(+), 82 deletions(-) diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp index 8503b41a42e5..7c5c5909d606 100644 --- a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp +++ b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp @@ -226,7 +226,9 @@ void EngineInfoContainerDBServerServerBased::addNode(ExecutionNode* node) { case ExecutionNode::TRAVERSAL: case ExecutionNode::SHORTEST_PATH: case ExecutionNode::K_SHORTEST_PATHS: { - injectVertexColletions(static_cast(node)); + auto* const graphNode = ExecutionNode::castTo(node); + graphNode->prepareOptions(); + injectVertexColletions(graphNode); break; } default: diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index 21103ce53012..c826921cfef2 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -2462,4 +2462,27 @@ void ExecutionPlan::show() const { _root->walk(shower); } +void ExecutionPlan::prepareTraversalOptions() { + ::arangodb::containers::SmallVector::allocator_type::arena_type a; + ::arangodb::containers::SmallVector nodes{a}; + findNodesOfType(nodes, + {arangodb::aql::ExecutionNode::TRAVERSAL, + arangodb::aql::ExecutionNode::SHORTEST_PATH, + arangodb::aql::ExecutionNode::K_SHORTEST_PATHS}, + true); + for (auto& node : nodes) { + switch (node->getType()) { + case ExecutionNode::TRAVERSAL: + case ExecutionNode::SHORTEST_PATH: + case ExecutionNode::K_SHORTEST_PATHS: { + auto* graphNode = ExecutionNode::castTo(node); + graphNode->prepareOptions(); + } break; + default: + TRI_ASSERT(false); + THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL_AQL); + } + } +} + #endif diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h index 6cb18be5ca96..11919f464612 100644 --- a/arangod/Aql/ExecutionPlan.h +++ b/arangod/Aql/ExecutionPlan.h @@ -196,6 +196,8 @@ class ExecutionPlan { /// @brief static analysis void planRegisters() { _root->planRegisters(); } + void prepareTraversalOptions(); + /// @brief unlinkNodes, note that this does not delete the removed /// nodes and that one cannot remove the root node of the plan. void unlinkNodes(std::unordered_set const& toUnlink); diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index ff25e1037934..8d8814cf4e49 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -387,6 +387,7 @@ GraphNode::GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& bas GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, + TRI_edge_direction_e defaultDirection, std::vector directions, std::unique_ptr options) : ExecutionNode(plan, id), @@ -397,6 +398,7 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, _tmpObjVariable(_plan->getAst()->variables()->createTemporaryVariable()), _tmpObjVarNode(_plan->getAst()->createNodeReference(_tmpObjVariable)), _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), + _defaultDirection(defaultDirection), _directions(std::move(directions)), _options(std::move(options)), _optionsBuilt(false), @@ -434,6 +436,7 @@ GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, _tmpObjVariable(_plan->getAst()->variables()->createTemporaryVariable()), _tmpObjVarNode(_plan->getAst()->createNodeReference(_tmpObjVariable)), _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), + _defaultDirection(other._defaultDirection), _directions(other._directions), _options(std::move(options)), _optionsBuilt(false), diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 94711afc6e3f..36619abd81a4 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -64,7 +64,7 @@ class GraphNode : public ExecutionNode { GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, - std::vector directions, + TRI_edge_direction_e defaultDirection, std::vector directions, std::unique_ptr options); /// @brief Clone constructor, used for constructors of derived classes. diff --git a/arangod/Aql/KShortestPathsNode.cpp b/arangod/Aql/KShortestPathsNode.cpp index d6ae8d41708e..9c03d4070801 100644 --- a/arangod/Aql/KShortestPathsNode.cpp +++ b/arangod/Aql/KShortestPathsNode.cpp @@ -144,10 +144,12 @@ KShortestPathsNode::KShortestPathsNode( ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, + TRI_edge_direction_e defaultDirection, std::vector const& directions, Variable const* inStartVariable, std::string const& startVertexId, Variable const* inTargetVariable, std::string const& targetVertexId, std::unique_ptr options) - : GraphNode(plan, id, vocbase, edgeColls, vertexColls, directions, std::move(options)), + : GraphNode(plan, id, vocbase, edgeColls, vertexColls, defaultDirection, + directions, std::move(options)), _pathOutVariable(nullptr), _inStartVariable(inStartVariable), _startVertexId(startVertexId), @@ -297,7 +299,8 @@ ExecutionNode* KShortestPathsNode::clone(ExecutionPlan* plan, bool withDependenc auto oldOpts = static_cast(options()); std::unique_ptr tmp = std::make_unique(*oldOpts); auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, - _vertexColls, _directions, _inStartVariable, + _vertexColls, _defaultDirection, + _directions, _inStartVariable, _startVertexId, _inTargetVariable, _targetVertexId, std::move(tmp)); if (usesPathOutVariable()) { diff --git a/arangod/Aql/KShortestPathsNode.h b/arangod/Aql/KShortestPathsNode.h index 16c1aa354747..0d7c793d5f3d 100644 --- a/arangod/Aql/KShortestPathsNode.h +++ b/arangod/Aql/KShortestPathsNode.h @@ -60,6 +60,7 @@ class KShortestPathsNode : public GraphNode { KShortestPathsNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, + TRI_edge_direction_e defaultDirection, std::vector const& directions, Variable const* inStartVariable, std::string const& startVertexId, Variable const* inTargetVariable, std::string const& targetVertexId, diff --git a/arangod/Aql/OptimizerRule.h b/arangod/Aql/OptimizerRule.h index 4330f62839dd..46694b54c3bd 100644 --- a/arangod/Aql/OptimizerRule.h +++ b/arangod/Aql/OptimizerRule.h @@ -217,7 +217,6 @@ struct OptimizerRule { // remove now obsolete path variables removeTraversalPathVariable, - prepareTraversalsRule, // when we have single document operations, fill in special cluster // handling. diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 7d324daba6d6..f94de7a39fe6 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -5944,42 +5944,6 @@ void arangodb::aql::removeTraversalPathVariable(Optimizer* opt, opt->addPlan(std::move(plan), rule, modified); } -/// @brief prepares traversals for execution (hidden rule) -void arangodb::aql::prepareTraversalsRule(Optimizer* opt, - std::unique_ptr plan, - OptimizerRule const& rule) { - ::arangodb::containers::SmallVector::allocator_type::arena_type a; - ::arangodb::containers::SmallVector tNodes{a}; - plan->findNodesOfType(tNodes, EN::TRAVERSAL, true); - plan->findNodesOfType(tNodes, EN::K_SHORTEST_PATHS, true); - plan->findNodesOfType(tNodes, EN::SHORTEST_PATH, true); - - if (tNodes.empty()) { - // no traversals present - opt->addPlan(std::move(plan), rule, false); - return; - } - - // first make a pass over all traversal nodes and remove unused - // variables from them - for (auto const& n : tNodes) { - if (n->getType() == EN::TRAVERSAL) { - TraversalNode* traversal = ExecutionNode::castTo(n); - traversal->prepareOptions(); - } else if (n->getType() == EN::K_SHORTEST_PATHS) { - TRI_ASSERT(n->getType() == EN::K_SHORTEST_PATHS); - KShortestPathsNode* spn = ExecutionNode::castTo(n); - spn->prepareOptions(); - } else { - TRI_ASSERT(n->getType() == EN::SHORTEST_PATH); - ShortestPathNode* spn = ExecutionNode::castTo(n); - spn->prepareOptions(); - } - } - - opt->addPlan(std::move(plan), rule, true); -} - /// @brief pulls out simple subqueries and merges them with the level above /// /// For example, if we have the input query diff --git a/arangod/Aql/OptimizerRules.h b/arangod/Aql/OptimizerRules.h index 13ea878061e3..e0f894b4f298 100644 --- a/arangod/Aql/OptimizerRules.h +++ b/arangod/Aql/OptimizerRules.h @@ -246,10 +246,6 @@ void removeFiltersCoveredByTraversal(Optimizer* opt, std::unique_ptr plan, OptimizerRule const&); -/// @brief prepares traversals for execution (hidden rule) -void prepareTraversalsRule(Optimizer* opt, std::unique_ptr plan, - OptimizerRule const&); - /// @brief moves simple subqueries one level higher void inlineSubqueriesRule(Optimizer*, std::unique_ptr, OptimizerRule const&); diff --git a/arangod/Aql/OptimizerRulesFeature.cpp b/arangod/Aql/OptimizerRulesFeature.cpp index db22a1e396c1..e788a7ca7531 100644 --- a/arangod/Aql/OptimizerRulesFeature.cpp +++ b/arangod/Aql/OptimizerRulesFeature.cpp @@ -305,10 +305,6 @@ void OptimizerRulesFeature::addRules() { OptimizerRule::removeTraversalPathVariable, OptimizerRule::makeFlags(OptimizerRule::Flags::CanBeDisabled)); - // prepare traversal info - registerRule("prepare-traversals", prepareTraversalsRule, OptimizerRule::prepareTraversalsRule, - OptimizerRule::makeFlags(OptimizerRule::Flags::Hidden)); - registerRule("optimize-cluster-single-document-operations", substituteClusterSingleDocumentOperationsRule, OptimizerRule::substituteSingleDocumentOperations, diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 89e85ec54cf4..855b622f40bf 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -1159,6 +1159,8 @@ QueryResult Query::explain() { plan->findVarUsage(); plan->planRegisters(); + plan->prepareTraversalOptions(); + plan->toVelocyPack(*result.data.get(), parser.ast(), _queryOptions.verbosePlans); } } @@ -1171,6 +1173,7 @@ QueryResult Query::explain() { bestPlan->findVarUsage(); bestPlan->planRegisters(); + bestPlan->prepareTraversalOptions(); result.data = bestPlan->toVelocyPack(parser.ast(), _queryOptions.verbosePlans); diff --git a/arangod/Aql/ShortestPathNode.cpp b/arangod/Aql/ShortestPathNode.cpp index 8d8cf3e2dcc8..736b595e6201 100644 --- a/arangod/Aql/ShortestPathNode.cpp +++ b/arangod/Aql/ShortestPathNode.cpp @@ -142,10 +142,12 @@ ShortestPathNode::ShortestPathNode( ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, + TRI_edge_direction_e defaultDirection, std::vector const& directions, Variable const* inStartVariable, std::string const& startVertexId, Variable const* inTargetVariable, std::string const& targetVertexId, std::unique_ptr options) - : GraphNode(plan, id, vocbase, edgeColls, vertexColls, directions, std::move(options)), + : GraphNode(plan, id, vocbase, edgeColls, vertexColls, defaultDirection, + directions, std::move(options)), _inStartVariable(inStartVariable), _startVertexId(startVertexId), _inTargetVariable(inTargetVariable), @@ -303,7 +305,8 @@ ExecutionNode* ShortestPathNode::clone(ExecutionPlan* plan, bool withDependencie auto oldOpts = static_cast(options()); std::unique_ptr tmp = std::make_unique(*oldOpts); auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, - _vertexColls, _directions, _inStartVariable, + _vertexColls, _defaultDirection, + _directions, _inStartVariable, _startVertexId, _inTargetVariable, _targetVertexId, std::move(tmp)); if (usesVertexOutVariable()) { diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index 7f5be5be1c4b..df8a71c81730 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -58,6 +58,7 @@ class ShortestPathNode : public GraphNode { ShortestPathNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, std::vector> const& edgeColls, std::vector> const& vertexColls, + TRI_edge_direction_e defaultDirection, std::vector const& directions, Variable const* inStartVariable, std::string const& startVertexId, Variable const* inTargetVariable, std::string const& targetVertexId, diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index a7afdae4249d..a2a80d85ef6e 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -165,9 +165,11 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocb std::vector> const& edgeColls, std::vector> const& vertexColls, Variable const* inVariable, std::string const& vertexId, + TRI_edge_direction_e defaultDirection, std::vector const& directions, std::unique_ptr options) - : GraphNode(plan, id, vocbase, edgeColls, vertexColls, directions, std::move(options)), + : GraphNode(plan, id, vocbase, edgeColls, vertexColls, defaultDirection, + directions, std::move(options)), _pathOutVariable(nullptr), _inVariable(inVariable), _vertexId(vertexId), @@ -274,12 +276,16 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co } TraversalNode::TraversalNode(ExecutionPlan& plan, TraversalNode const& other) - : GraphNode(plan, other, std::make_unique(*options())), + : GraphNode(plan, other, + std::make_unique(*other.options())), _pathOutVariable(nullptr), _inVariable(other._inVariable), _vertexId(other._vertexId), _fromCondition(nullptr), - _toCondition(nullptr) {} + _toCondition(nullptr) { + TRI_ASSERT(!other._optionsBuilt); + other.traversalCloneHelper(plan, *this, false); +} TraversalNode::~TraversalNode() = default; @@ -537,40 +543,47 @@ ExecutionNode* TraversalNode::clone(ExecutionPlan* plan, bool withDependencies, TRI_ASSERT(!_optionsBuilt); auto oldOpts = options(); std::unique_ptr tmp = std::make_unique(*oldOpts); - auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, - _vertexColls, _inVariable, _vertexId, + auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, _vertexColls, + _inVariable, _vertexId, _defaultDirection, _directions, std::move(tmp)); + traversalCloneHelper(*plan, *c, withProperties); + + return cloneHelper(std::move(c), withDependencies, withProperties); +} + +void TraversalNode::traversalCloneHelper(ExecutionPlan& plan, TraversalNode& c, + bool const withProperties) const { if (usesVertexOutVariable()) { auto vertexOutVariable = _vertexOutVariable; if (withProperties) { - vertexOutVariable = plan->getAst()->variables()->createVariable(vertexOutVariable); + vertexOutVariable = plan.getAst()->variables()->createVariable(vertexOutVariable); } TRI_ASSERT(vertexOutVariable != nullptr); - c->setVertexOutput(vertexOutVariable); + c.setVertexOutput(vertexOutVariable); } if (usesEdgeOutVariable()) { auto edgeOutVariable = _edgeOutVariable; if (withProperties) { - edgeOutVariable = plan->getAst()->variables()->createVariable(edgeOutVariable); + edgeOutVariable = plan.getAst()->variables()->createVariable(edgeOutVariable); } TRI_ASSERT(edgeOutVariable != nullptr); - c->setEdgeOutput(edgeOutVariable); + c.setEdgeOutput(edgeOutVariable); } if (usesPathOutVariable()) { auto pathOutVariable = _pathOutVariable; if (withProperties) { - pathOutVariable = plan->getAst()->variables()->createVariable(pathOutVariable); + pathOutVariable = plan.getAst()->variables()->createVariable(pathOutVariable); } TRI_ASSERT(pathOutVariable != nullptr); - c->setPathOutput(pathOutVariable); + c.setPathOutput(pathOutVariable); } - c->_conditionVariables.reserve(_conditionVariables.size()); + c._conditionVariables.reserve(_conditionVariables.size()); for (auto const& it : _conditionVariables) { - c->_conditionVariables.emplace(it->clone()); + c._conditionVariables.emplace(it->clone()); } #ifdef ARANGODB_ENABLE_MAINTAINER_MODE @@ -578,36 +591,34 @@ ExecutionNode* TraversalNode::clone(ExecutionPlan* plan, bool withDependencies, #endif // Temporary Filter Objects - c->_tmpObjVariable = _tmpObjVariable; - c->_tmpObjVarNode = _tmpObjVarNode; - c->_tmpIdNode = _tmpIdNode; + c._tmpObjVariable = _tmpObjVariable; + c._tmpObjVarNode = _tmpObjVarNode; + c._tmpIdNode = _tmpIdNode; // Filter Condition Parts - c->_fromCondition = _fromCondition->clone(_plan->getAst()); - c->_toCondition = _toCondition->clone(_plan->getAst()); - c->_globalEdgeConditions.insert(c->_globalEdgeConditions.end(), - _globalEdgeConditions.begin(), - _globalEdgeConditions.end()); - c->_globalVertexConditions.insert(c->_globalVertexConditions.end(), - _globalVertexConditions.begin(), - _globalVertexConditions.end()); + c._fromCondition = _fromCondition->clone(_plan->getAst()); + c._toCondition = _toCondition->clone(_plan->getAst()); + c._globalEdgeConditions.insert(c._globalEdgeConditions.end(), + _globalEdgeConditions.begin(), + _globalEdgeConditions.end()); + c._globalVertexConditions.insert(c._globalVertexConditions.end(), + _globalVertexConditions.begin(), + _globalVertexConditions.end()); for (auto const& it : _edgeConditions) { // Copy the builder auto ecBuilder = std::make_unique(this, it.second.get()); - c->_edgeConditions.try_emplace(it.first, std::move(ecBuilder)); + c._edgeConditions.try_emplace(it.first, std::move(ecBuilder)); } for (auto const& it : _vertexConditions) { - c->_vertexConditions.try_emplace(it.first, it.second->clone(_plan->getAst())); + c._vertexConditions.try_emplace(it.first, it.second->clone(_plan->getAst())); } #ifdef ARANGODB_ENABLE_MAINTAINER_MODE - c->checkConditionsDefined(); + c.checkConditionsDefined(); #endif - - return cloneHelper(std::move(c), withDependencies, withProperties); } void TraversalNode::prepareOptions() { diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index 1e898757dd3d..a9e8df9b5b13 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -87,9 +87,10 @@ class TraversalNode : public GraphNode { /// @brief Internal constructor to clone the node. TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, - std::vector> const& edgeColls, - std::vector> const& vertexColls, + std::vector> const& edgeColls, + std::vector> const& vertexColls, Variable const* inVariable, std::string const& vertexId, + TRI_edge_direction_e defaultDirection, std::vector const& directions, std::unique_ptr options); @@ -212,6 +213,8 @@ class TraversalNode : public GraphNode { void checkConditionsDefined() const; #endif + void traversalCloneHelper(ExecutionPlan& plan, TraversalNode& c, bool withProperties) const; + private: /// @brief vertex output variable Variable const* _pathOutVariable; From adfe02c32e4db3fec6b754707924252e6da41d2a Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 29 Jan 2020 20:11:18 +0100 Subject: [PATCH 19/47] ui - more satellite graph preparation, additional cleanup --- .../frontend/js/views/graphManagementView.js | 229 ++++++++++-------- 1 file changed, 134 insertions(+), 95 deletions(-) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js index 3d1c32ad4819..5c9907a77858 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js @@ -44,35 +44,52 @@ } if (id === 'smartGraph') { - this.toggleSmartGraph(); - $('#createGraph').addClass('active'); - this.showSmartGraphOptions(); + // TODO: to be implemented properly + // add css class to entry + $('#createGraph').addClass('active'); // TODO: check addClass? + this.setCacheModeState(true); + this.showSmartGraphRows(); + } else if (id === 'satelliteGraph') { + // TODO: to be implemented properly + this.setCacheModeState(true); + this.hideSmartGraphRows(); } else if (id === 'createGraph') { - this.toggleSmartGraph(); - this.hideSmartGraphOptions(); + // TODO: to be implemented properly + this.setCacheModeState(false); + this.hideSmartGraphRows(); + this.hideSatelliteGraphRows(); } }, - hideSmartGraphOptions: function () { - $('#row_general-numberOfShards').show(); - $('#row_general-replicationFactor').show(); - $('#row_general-writeConcern').show(); - $('#smartGraphInfo').hide(); - $('#row_new-numberOfShards').hide(); - $('#row_new-replicationFactor').hide(); - $('#row_new-writeConcern').hide(); - $('#row_new-smartGraphAttribute').hide(); + smartGraphRows: [ + 'row_general-numberOfShards', + 'row_general-replicationFactor', + 'row_general-writeConcern', + 'smartGraphInfo', + 'row_new-numberOfShards', + 'row_new-replicationFactor', + 'row_new-writeConcern', + 'row_new-smartGraphAttribute' + ], + + hideSatelliteGraphRows: function () { + // TODO: to be implemented }, - showSmartGraphOptions: function () { - $('#row_general-numberOfShards').hide(); - $('#row_general-replicationFactor').hide(); - $('#row_general-writeConcern').hide(); - $('#smartGraphInfo').show(); - $('#row_new-numberOfShards').show(); - $('#row_new-replicationFactor').show(); - $('#row_new-writeConcern').show(); - $('#row_new-smartGraphAttribute').show(); + showSatelliteGraphRows: function () { + // TODO: to be implemented + }, + + hideSmartGraphRows: function () { + _.each(this.smartGraphRows, function (rowId) { + $('#' + rowId).hide(); + }); + }, + + showSmartGraphRows: function () { + _.each(this.smartGraphRows, function (rowId) { + $('#' + rowId).show(); + }); }, redirectToGraphViewer: function (e) { @@ -146,8 +163,11 @@ this.createEditGraphModal(); } else { this.createEditGraphModal(); - // hide tab entry + // hide tab entries + // no smart graphs in single server mode $('#tab-smartGraph').parent().remove(); + // no satellite graphs in single server mode + $('#tab-satelliteGraph').parent().remove(); } } }, @@ -250,85 +270,99 @@ }); }, - toggleSmartGraph: function () { - if (!frontendConfig.isCluster || !frontendConfig.isEnterprise) { - return; - } + forgetCachedCollectionsState: function () { + // Note: re-enable cached collections for general graph + // General graph collections are allowed to use existing collections + // Satellite Graphs and Smart Graphs are not allowed to use them, so we need to "forget" them here + var collList = []; + var self = this; + var collections = this.options.collectionCollection.models; + + collections.forEach(function (c) { + if (c.get('isSystem')) { + return; + } + collList.push(c.id); + }); var i; + for (i = 0; i < this.counter; i++) { + $('#newEdgeDefinitions' + i).select2({ + tags: self.eCollList + }); + $('#newEdgeDefinitions' + i).select2('data', self.cachedNewEdgeDefinitions); + $('#newEdgeDefinitions' + i).attr('disabled', self.cachedNewEdgeDefinitionsState); + + $('#fromCollections' + i).select2({ + tags: collList + }); + $('#fromCollections' + i).select2('data', self.cachedFromCollections); + $('#fromCollections' + i).attr('disabled', self.cachedFromCollectionsState); + + $('#toCollections' + i).select2({ + tags: collList + }); + $('#toCollections' + i).select2('data', self.cachedToCollections); + $('#toCollections' + i).attr('disabled', self.cachedToCollectionsState); + } + $('#newVertexCollections').select2({ + tags: collList + }); + $('#newVertexCollections').select2('data', self.cachedNewVertexCollections); + $('#newVertexCollections').attr('disabled', self.cachedNewVertexCollectionsState); + }, + + rememberCachedCollectionsState: function () { var self = this; + var i; - if (!$('#tab-smartGraph').parent().hasClass('active')) { - for (i = 0; i < this.counter; i++) { - $('#newEdgeDefinitions' + i).select2({ - tags: [] - }); - self.cachedNewEdgeDefinitions = $('#newEdgeDefinitions' + i).select2('data'); - self.cachedNewEdgeDefinitionsState = $('#newEdgeDefinitions' + i).attr('disabled'); - $('#newEdgeDefinitions' + i).select2('data', ''); - $('#newEdgeDefinitions' + i).attr('disabled', false); - $('#newEdgeDefinitions' + i).change(); - - $('#fromCollections' + i).select2({ - tags: [] - }); - self.cachedFromCollections = $('#fromCollections' + i).select2('data'); - self.cachedFromCollectionsState = $('#fromCollections' + i).attr('disabled'); - $('#fromCollections' + i).select2('data', ''); - $('#fromCollections' + i).attr('disabled', false); - $('#fromCollections' + i).change(); - - $('#toCollections' + i).select2({ - tags: [] - }); - self.cachedToCollections = $('#toCollections' + i).select2('data'); - self.cachedToCollectionsState = $('#toCollections' + i).attr('disabled'); - $('#toCollections' + i).select2('data', ''); - $('#toCollections' + i).attr('disabled', false); - $('#toCollections' + i).change(); - } - $('#newVertexCollections').select2({ + for (i = 0; i < self.counter; i++) { + $('#newEdgeDefinitions' + i).select2({ tags: [] }); - self.cachedNewVertexCollections = $('#newVertexCollections').select2('data'); - self.cachedNewVertexCollectionsState = $('#newVertexCollections').attr('disabled'); - $('#newVertexCollections').select2('data', ''); - $('#newVertexCollections').attr('disabled', false); - $('#newVertexCollections').change(); - } else { - var collList = []; var collections = this.options.collectionCollection.models; + self.cachedNewEdgeDefinitions = $('#newEdgeDefinitions' + i).select2('data'); + self.cachedNewEdgeDefinitionsState = $('#newEdgeDefinitions' + i).attr('disabled'); + $('#newEdgeDefinitions' + i).select2('data', ''); + $('#newEdgeDefinitions' + i).attr('disabled', false); + $('#newEdgeDefinitions' + i).change(); - collections.forEach(function (c) { - if (c.get('isSystem')) { - return; - } - collList.push(c.id); + $('#fromCollections' + i).select2({ + tags: [] }); + self.cachedFromCollections = $('#fromCollections' + i).select2('data'); + self.cachedFromCollectionsState = $('#fromCollections' + i).attr('disabled'); + $('#fromCollections' + i).select2('data', ''); + $('#fromCollections' + i).attr('disabled', false); + $('#fromCollections' + i).change(); - for (i = 0; i < this.counter; i++) { - $('#newEdgeDefinitions' + i).select2({ - tags: this.eCollList - }); - $('#newEdgeDefinitions' + i).select2('data', self.cachedNewEdgeDefinitions); - $('#newEdgeDefinitions' + i).attr('disabled', self.cachedNewEdgeDefinitionsState); - - $('#fromCollections' + i).select2({ - tags: collList - }); - $('#fromCollections' + i).select2('data', self.cachedFromCollections); - $('#fromCollections' + i).attr('disabled', self.cachedFromCollectionsState); - - $('#toCollections' + i).select2({ - tags: collList - }); - $('#toCollections' + i).select2('data', self.cachedToCollections); - $('#toCollections' + i).attr('disabled', self.cachedToCollectionsState); - } - $('#newVertexCollections').select2({ - tags: collList + $('#toCollections' + i).select2({ + tags: [] }); - $('#newVertexCollections').select2('data', self.cachedNewVertexCollections); - $('#newVertexCollections').attr('disabled', self.cachedNewVertexCollectionsState); + self.cachedToCollections = $('#toCollections' + i).select2('data'); + self.cachedToCollectionsState = $('#toCollections' + i).attr('disabled'); + $('#toCollections' + i).select2('data', ''); + $('#toCollections' + i).attr('disabled', false); + $('#toCollections' + i).change(); + } + $('#newVertexCollections').select2({ + tags: [] + }); + self.cachedNewVertexCollections = $('#newVertexCollections').select2('data'); + self.cachedNewVertexCollectionsState = $('#newVertexCollections').attr('disabled'); + $('#newVertexCollections').select2('data', ''); + $('#newVertexCollections').attr('disabled', false); + $('#newVertexCollections').change(); + }, + + setCacheModeState: function (forget) { + if (!frontendConfig.isCluster || !frontendConfig.isEnterprise) { + return; + } + + if (forget) { + this.forgetCachedCollectionsState(); + } else { + this.rememberCachedCollectionsState(); } }, @@ -431,6 +465,8 @@ var graph = this.collection.findWhere({_key: this.graphToEdit}); if (graph.get('isSmart')) { this.createEditGraphModal(graph, true); + } else if (graph.get('replicationFactor') === 'satellite') { + this.createEditGraphModal(graph, false, true); } else { this.createEditGraphModal(graph); } @@ -723,7 +759,7 @@ }); }, - createEditGraphModal: function (graph, isSmart) { + createEditGraphModal: function (graph, isSmart, isSatellite) { var buttons = []; var collList = []; var tableContent = []; @@ -763,6 +799,8 @@ if (graph) { if (isSmart) { title = 'Edit Smart Graph'; + } else if (isSatelite) { + title = 'Edit Satellite Graph'; } else { title = 'Edit Graph'; } @@ -1068,7 +1106,8 @@ ); if ($('#tab-createGraph').parent().hasClass('active')) { - self.hideSmartGraphOptions(); + // hide them by default, as we're showing general graph as default + self.hideSmartGraphRows(); } if (graph) { From 02432782331aed7055f68828007b8b182513d06c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 30 Jan 2020 14:06:55 +0100 Subject: [PATCH 20/47] Fixed query snippets --- arangod/Aql/GraphNode.cpp | 14 +++++++++++ arangod/Aql/GraphNode.h | 4 ++- arangod/Aql/QuerySnippet.cpp | 48 +++++++++++++++++++++++++++--------- arangod/Aql/ShardLocking.cpp | 13 +++++----- 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index 8d8814cf4e49..989467f94a9c 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -35,8 +35,12 @@ #include "Cluster/ServerState.h" #include "Graph/BaseOptions.h" #include "Graph/Graph.h" +#include "TraversalNode.h" #include "Utils/CollectionNameResolver.h" #include "VocBase/LogicalCollection.h" +#ifdef USE_ENTERPRISE +#include "Enterprise/Aql/SatelliteTraversalNode.h" +#endif #include @@ -683,3 +687,13 @@ std::vector> const& GraphNode::vertexColls() co graph::Graph const* GraphNode::graph() const noexcept { return _graphObj; } + +bool GraphNode::useAsSatellite() const { +#ifndef USE_ENTERPRISE + return false; +#else + auto const* traversalNode = dynamic_cast(this); + // TODO do this for shortest path and k-shortest paths as well + return traversalNode != nullptr; +#endif +} diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 36619abd81a4..22fea30e79da 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -59,6 +59,8 @@ class GraphNode : public ExecutionNode { GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base); + bool useAsSatellite() const; + protected: /// @brief Internal constructor to clone the node. GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, @@ -213,7 +215,7 @@ class GraphNode : public ExecutionNode { /// @brief flag, if graph is smart (enterprise edition only!) bool _isSmart; - /// @brief list of shards involved, requried for one-shard-databases + /// @brief list of shards involved, required for one-shard-databases std::map _collectionToShard; }; diff --git a/arangod/Aql/QuerySnippet.cpp b/arangod/Aql/QuerySnippet.cpp index 3038df860c58..124c7b720710 100644 --- a/arangod/Aql/QuerySnippet.cpp +++ b/arangod/Aql/QuerySnippet.cpp @@ -34,7 +34,6 @@ #include "Basics/StringUtils.h" #include "Cluster/ServerState.h" -#include #include using namespace arangodb; @@ -64,7 +63,9 @@ void QuerySnippet::addNode(ExecutionNode* node) { case ExecutionNode::TRAVERSAL: case ExecutionNode::SHORTEST_PATH: case ExecutionNode::K_SHORTEST_PATHS: { - _expansions.emplace_back(node, false, false); + auto* graphNode = ExecutionNode::castTo(node); + auto const isSatellite = graphNode->useAsSatellite(); + _expansions.emplace_back(node, false, isSatellite); break; } case ExecutionNode::ENUMERATE_IRESEARCH_VIEW: { @@ -341,25 +342,48 @@ ResultT>> QuerySnippet::pre // the same translation is copied to all servers // there are no local expansions - GraphNode* graphNode = ExecutionNode::castTo(exp.node); + auto* graphNode = ExecutionNode::castTo(exp.node); graphNode->setCollectionToShard({}); //clear previous information - for(auto* aqlCollection : graphNode->collections()) { - auto const& shards = aqlCollection->shardIds(); - TRI_ASSERT(!shards->empty()); - for (std::string const& shard : *shards) { - auto found = shardMapping.find(shard); - if (found != shardMapping.end() && found->second == server) { - // provide a correct translation from collection to shard - // to be used in toVelocyPack methods of classes derived - // from GraphNode + TRI_ASSERT(graphNode->useAsSatellite() == exp.isSatellite); + + if (!exp.isSatellite) { + for (auto* aqlCollection : graphNode->collections()) { + auto const& shards = aqlCollection->shardIds(); + TRI_ASSERT(!shards->empty()); + for (std::string const& shard : *shards) { + auto found = shardMapping.find(shard); + if (found != shardMapping.end() && found->second == server) { + // provide a correct translation from collection to shard + // to be used in toVelocyPack methods of classes derived + // from GraphNode + graphNode->addCollectionToShard(aqlCollection->name(), shard); + } + } + } + } else { + for (auto* aqlCollection : graphNode->collections()) { + auto const& shards = shardLocking.shardsForSnippet(id(), aqlCollection); + for (auto const& shard : shards) { + // If we find a shard here that is not in this mapping, + // we have 1) a problem with locking before that should have thrown + // 2) a problem with shardMapping lookup that should have thrown before + TRI_ASSERT(shardMapping.find(shard) != shardMapping.end()); + // Could we replace the outer if/else on isSatellite with this branch + // here, and remove the upper part? + // if (check->second == server || exp.isSatellite) {...} + graphNode->addCollectionToShard(aqlCollection->name(), shard); + + // This case currently does not exist and is not handled here. + TRI_ASSERT(!exp.doExpand); } } } continue; // skip rest - there are no local expansions } + // exp.node is now either an enumerate collection, index, or modification. // It is of utmost importance that this is an ordered set of Shards. // We can only join identical indexes of shards for each collection diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 260d7033c1ff..48e5e8e56acd 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -55,20 +55,21 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId) { case ExecutionNode::SHORTEST_PATH: case ExecutionNode::TRAVERSAL: { // Add GraphNode - auto node = ExecutionNode::castTo(baseNode); - if (node == nullptr) { + auto* graphNode = ExecutionNode::castTo(baseNode); + if (graphNode == nullptr) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "unable to cast node to GraphNode"); } + auto const useAsSatellite = graphNode->useAsSatellite(); // Add all Edge Collections to the Transactions, Traversals do never write - for (auto const& col : node->edgeColls()) { - updateLocking(col.get(), AccessMode::Type::READ, snippetId, {}, false); + for (auto const& col : graphNode->edgeColls()) { + updateLocking(col.get(), AccessMode::Type::READ, snippetId, {}, useAsSatellite); } // Add all Vertex Collections to the Transactions, Traversals do never // write, the collections have been adjusted already - for (auto const& col : node->vertexColls()) { - updateLocking(col.get(), AccessMode::Type::READ, snippetId, {}, false); + for (auto const& col : graphNode->vertexColls()) { + updateLocking(col.get(), AccessMode::Type::READ, snippetId, {}, useAsSatellite); } break; } From e1e61d48846fac05d47e9a59b9eb07c1d13b07f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 30 Jan 2020 15:29:00 +0100 Subject: [PATCH 21/47] Fix non-maintainer build --- arangod/Aql/ExecutionPlan.cpp | 46 ++++++++++++------------ arangod/Aql/TraversalConditionFinder.cpp | 2 +- arangod/Aql/TraversalNode.cpp | 26 +++++++------- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index c826921cfef2..4a87e0076b84 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -2417,6 +2417,29 @@ bool ExecutionPlan::fullCount() const noexcept { return lastLimitNode != nullptr && lastLimitNode->fullCount(); } +void ExecutionPlan::prepareTraversalOptions() { + ::arangodb::containers::SmallVector::allocator_type::arena_type a; + ::arangodb::containers::SmallVector nodes{a}; + findNodesOfType(nodes, + {arangodb::aql::ExecutionNode::TRAVERSAL, + arangodb::aql::ExecutionNode::SHORTEST_PATH, + arangodb::aql::ExecutionNode::K_SHORTEST_PATHS}, + true); + for (auto& node : nodes) { + switch (node->getType()) { + case ExecutionNode::TRAVERSAL: + case ExecutionNode::SHORTEST_PATH: + case ExecutionNode::K_SHORTEST_PATHS: { + auto* graphNode = ExecutionNode::castTo(node); + graphNode->prepareOptions(); + } break; + default: + TRI_ASSERT(false); + THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL_AQL); + } + } +} + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE #include @@ -2462,27 +2485,4 @@ void ExecutionPlan::show() const { _root->walk(shower); } -void ExecutionPlan::prepareTraversalOptions() { - ::arangodb::containers::SmallVector::allocator_type::arena_type a; - ::arangodb::containers::SmallVector nodes{a}; - findNodesOfType(nodes, - {arangodb::aql::ExecutionNode::TRAVERSAL, - arangodb::aql::ExecutionNode::SHORTEST_PATH, - arangodb::aql::ExecutionNode::K_SHORTEST_PATHS}, - true); - for (auto& node : nodes) { - switch (node->getType()) { - case ExecutionNode::TRAVERSAL: - case ExecutionNode::SHORTEST_PATH: - case ExecutionNode::K_SHORTEST_PATHS: { - auto* graphNode = ExecutionNode::castTo(node); - graphNode->prepareOptions(); - } break; - default: - TRI_ASSERT(false); - THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL_AQL); - } - } -} - #endif diff --git a/arangod/Aql/TraversalConditionFinder.cpp b/arangod/Aql/TraversalConditionFinder.cpp index c9bf994aed3c..cf619eb6a5a5 100644 --- a/arangod/Aql/TraversalConditionFinder.cpp +++ b/arangod/Aql/TraversalConditionFinder.cpp @@ -571,7 +571,7 @@ bool TraversalConditionFinder::before(ExecutionNode* en) { // No condition, no optimize break; } - auto options = static_cast(node->options()); + auto options = node->options(); auto const& varsValidInTraversal = node->getVarsValid(); bool conditionIsImpossible = false; diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index a2a80d85ef6e..fdcc9ab0e416 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -785,19 +785,6 @@ void TraversalNode::getPruneVariables(std::vector& res) const { } } -#ifdef ARANGODB_ENABLE_MAINTAINER_MODE -void TraversalNode::checkConditionsDefined() const { - TRI_ASSERT(_tmpObjVariable != nullptr); - TRI_ASSERT(_tmpObjVarNode != nullptr); - TRI_ASSERT(_tmpIdNode != nullptr); - - TRI_ASSERT(_fromCondition != nullptr); - TRI_ASSERT(_fromCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); - - TRI_ASSERT(_toCondition != nullptr); - TRI_ASSERT(_toCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); -} - bool TraversalNode::isEligibleAsSatelliteTraversal() const { return graph() != nullptr && graph()->isSatellite(); } @@ -812,4 +799,17 @@ auto TraversalNode::options() const -> TraverserOptions* { return opts; } +#ifdef ARANGODB_ENABLE_MAINTAINER_MODE +void TraversalNode::checkConditionsDefined() const { + TRI_ASSERT(_tmpObjVariable != nullptr); + TRI_ASSERT(_tmpObjVarNode != nullptr); + TRI_ASSERT(_tmpIdNode != nullptr); + + TRI_ASSERT(_fromCondition != nullptr); + TRI_ASSERT(_fromCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); + + TRI_ASSERT(_toCondition != nullptr); + TRI_ASSERT(_toCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); +} + #endif From 0aa6acdfd36b029d375296ad22fcf43dad6ef7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 30 Jan 2020 16:38:24 +0100 Subject: [PATCH 22/47] Two bugfixes --- arangod/Aql/GraphNode.cpp | 4 ++-- arangod/Aql/GraphNode.h | 2 +- arangod/Aql/QuerySnippet.cpp | 4 ++-- arangod/Aql/ShardLocking.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index 989467f94a9c..ca3c35236a8a 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -688,12 +688,12 @@ graph::Graph const* GraphNode::graph() const noexcept { return _graphObj; } -bool GraphNode::useAsSatellite() const { +bool GraphNode::isUsedAsSatellite() const { #ifndef USE_ENTERPRISE return false; #else auto const* traversalNode = dynamic_cast(this); // TODO do this for shortest path and k-shortest paths as well - return traversalNode != nullptr; + return traversalNode != nullptr && traversalNode->isUsedAsSatellite(); #endif } diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 22fea30e79da..36fc2d7ca83e 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -59,7 +59,7 @@ class GraphNode : public ExecutionNode { GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base); - bool useAsSatellite() const; + bool isUsedAsSatellite() const; protected: /// @brief Internal constructor to clone the node. diff --git a/arangod/Aql/QuerySnippet.cpp b/arangod/Aql/QuerySnippet.cpp index 124c7b720710..b264592f1822 100644 --- a/arangod/Aql/QuerySnippet.cpp +++ b/arangod/Aql/QuerySnippet.cpp @@ -64,7 +64,7 @@ void QuerySnippet::addNode(ExecutionNode* node) { case ExecutionNode::SHORTEST_PATH: case ExecutionNode::K_SHORTEST_PATHS: { auto* graphNode = ExecutionNode::castTo(node); - auto const isSatellite = graphNode->useAsSatellite(); + auto const isSatellite = graphNode->isUsedAsSatellite(); _expansions.emplace_back(node, false, isSatellite); break; } @@ -345,7 +345,7 @@ ResultT>> QuerySnippet::pre auto* graphNode = ExecutionNode::castTo(exp.node); graphNode->setCollectionToShard({}); //clear previous information - TRI_ASSERT(graphNode->useAsSatellite() == exp.isSatellite); + TRI_ASSERT(graphNode->isUsedAsSatellite() == exp.isSatellite); if (!exp.isSatellite) { for (auto* aqlCollection : graphNode->collections()) { diff --git a/arangod/Aql/ShardLocking.cpp b/arangod/Aql/ShardLocking.cpp index 48e5e8e56acd..fae8abe92c9b 100644 --- a/arangod/Aql/ShardLocking.cpp +++ b/arangod/Aql/ShardLocking.cpp @@ -60,7 +60,7 @@ void ShardLocking::addNode(ExecutionNode const* baseNode, size_t snippetId) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "unable to cast node to GraphNode"); } - auto const useAsSatellite = graphNode->useAsSatellite(); + auto const useAsSatellite = graphNode->isUsedAsSatellite(); // Add all Edge Collections to the Transactions, Traversals do never write for (auto const& col : graphNode->edgeColls()) { updateLocking(col.get(), AccessMode::Type::READ, snippetId, {}, useAsSatellite); From afefc0c1141a7a4a9d780f9da72ca110a4e53a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Fri, 31 Jan 2020 13:12:59 +0100 Subject: [PATCH 23/47] Implemented shortest path --- arangod/Aql/GraphNode.cpp | 35 +++++++++++++++++++++-- arangod/Aql/GraphNode.h | 6 ++++ arangod/Aql/ShortestPathNode.cpp | 48 ++++++++++++++++++++++++-------- arangod/Aql/ShortestPathNode.h | 15 ++++++++++ arangod/Aql/TraversalNode.cpp | 5 +--- arangod/Aql/TraversalNode.h | 2 -- 6 files changed, 91 insertions(+), 20 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index ca3c35236a8a..5fa214b313a8 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -30,18 +30,38 @@ #include "ApplicationFeatures/ApplicationServer.h" #include "Aql/Ast.h" #include "Aql/Collection.h" +#include "Aql/ExecutionBlockImpl.h" +#include "Aql/ExecutionEngine.h" #include "Aql/ExecutionPlan.h" +#include "Aql/Expression.h" +#include "Aql/Query.h" +#include "Aql/RegisterPlan.h" +#include "Aql/SingleRowFetcher.h" +#include "Aql/SortCondition.h" +#include "Aql/TraversalExecutor.h" +#include "Aql/Variable.h" +#include "Basics/StringUtils.h" +#include "Basics/tryEmplaceHelper.h" #include "Cluster/ClusterFeature.h" +#include "Cluster/ClusterTraverser.h" #include "Cluster/ServerState.h" +#include "Enterprise/Cluster/SmartGraphTraverser.h" #include "Graph/BaseOptions.h" #include "Graph/Graph.h" +#include "Graph/SingleServerTraverser.h" +#include "Graph/TraverserOptions.h" +#include "Indexes/Index.h" #include "TraversalNode.h" #include "Utils/CollectionNameResolver.h" #include "VocBase/LogicalCollection.h" +#include "VocBase/ticks.h" #ifdef USE_ENTERPRISE #include "Enterprise/Aql/SatelliteTraversalNode.h" +#include "Enterprise/Aql/SatelliteShortestPathNode.h" #endif +#include +#include #include using namespace arangodb; @@ -692,8 +712,17 @@ bool GraphNode::isUsedAsSatellite() const { #ifndef USE_ENTERPRISE return false; #else - auto const* traversalNode = dynamic_cast(this); - // TODO do this for shortest path and k-shortest paths as well - return traversalNode != nullptr && traversalNode->isUsedAsSatellite(); + auto const* collectionAccessingNode = dynamic_cast(this); + TRI_ASSERT((collectionAccessingNode != nullptr) == + (nullptr != dynamic_cast(this) || + nullptr != dynamic_cast(this) + // TODO uncomment the next line when SatelliteKShortestPathsNode is implemented: + // || nullptr != dynamic_cast(this) + )); + return collectionAccessingNode != nullptr && collectionAccessingNode->isUsedAsSatellite(); #endif } + +bool GraphNode::isEligibleAsSatelliteTraversal() const { + return graph() != nullptr && graph()->isSatellite(); +} diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 36fc2d7ca83e..2ec0dacd4331 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -24,9 +24,13 @@ #ifndef ARANGOD_AQL_GRAPH_NODE_H #define ARANGOD_AQL_GRAPH_NODE_H 1 +#include "Aql/Condition.h" #include "Aql/ExecutionNode.h" +#include "Aql/GraphNode.h" +#include "Aql/Graphs.h" #include "Cluster/ClusterTypes.h" #include "Cluster/TraverserEngineRegistry.h" +#include "VocBase/LogicalCollection.h" #include "VocBase/voc-types.h" #include @@ -61,6 +65,8 @@ class GraphNode : public ExecutionNode { bool isUsedAsSatellite() const; + bool isEligibleAsSatelliteTraversal() const; + protected: /// @brief Internal constructor to clone the node. GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, diff --git a/arangod/Aql/ShortestPathNode.cpp b/arangod/Aql/ShortestPathNode.cpp index 736b595e6201..c2f5bc749e13 100644 --- a/arangod/Aql/ShortestPathNode.cpp +++ b/arangod/Aql/ShortestPathNode.cpp @@ -309,34 +309,39 @@ ExecutionNode* ShortestPathNode::clone(ExecutionPlan* plan, bool withDependencie _directions, _inStartVariable, _startVertexId, _inTargetVariable, _targetVertexId, std::move(tmp)); + shortestPathCloneHelper(*plan, *c, withProperties); + + return cloneHelper(std::move(c), withDependencies, withProperties); +} + +void ShortestPathNode::shortestPathCloneHelper(ExecutionPlan& plan, ShortestPathNode& c, + bool withProperties) const { if (usesVertexOutVariable()) { auto vertexOutVariable = _vertexOutVariable; if (withProperties) { - vertexOutVariable = plan->getAst()->variables()->createVariable(vertexOutVariable); + vertexOutVariable = plan.getAst()->variables()->createVariable(vertexOutVariable); } TRI_ASSERT(vertexOutVariable != nullptr); - c->setVertexOutput(vertexOutVariable); + c.setVertexOutput(vertexOutVariable); } if (usesEdgeOutVariable()) { auto edgeOutVariable = _edgeOutVariable; if (withProperties) { - edgeOutVariable = plan->getAst()->variables()->createVariable(edgeOutVariable); + edgeOutVariable = plan.getAst()->variables()->createVariable(edgeOutVariable); } TRI_ASSERT(edgeOutVariable != nullptr); - c->setEdgeOutput(edgeOutVariable); + c.setEdgeOutput(edgeOutVariable); } // Temporary Filter Objects - c->_tmpObjVariable = _tmpObjVariable; - c->_tmpObjVarNode = _tmpObjVarNode; - c->_tmpIdNode = _tmpIdNode; + c._tmpObjVariable = _tmpObjVariable; + c._tmpObjVarNode = _tmpObjVarNode; + c._tmpIdNode = _tmpIdNode; // Filter Condition Parts - c->_fromCondition = _fromCondition->clone(_plan->getAst()); - c->_toCondition = _toCondition->clone(_plan->getAst()); - - return cloneHelper(std::move(c), withDependencies, withProperties); + c._fromCondition = _fromCondition->clone(_plan->getAst()); + c._toCondition = _toCondition->clone(_plan->getAst()); } void ShortestPathNode::prepareOptions() { @@ -381,3 +386,24 @@ void ShortestPathNode::prepareOptions() { } _optionsBuilt = true; } + +auto ShortestPathNode::options() const -> ShortestPathOptions* { +#ifdef ARANGODB_ENABLE_MAINTAINER_MODE + auto* opts = dynamic_cast(GraphNode::options()); + TRI_ASSERT((GraphNode::options() == nullptr) == (opts == nullptr)); +#else + auto* opts = static_cast(GraphNode::options()); +#endif + return opts; +} + +ShortestPathNode::ShortestPathNode(ExecutionPlan& plan, const ShortestPathNode& other) + : GraphNode(plan, other, std::make_unique(*other.options())), + _inStartVariable(other._inStartVariable), + _startVertexId(other._startVertexId), + _inTargetVariable(other._inTargetVariable), + _targetVertexId(other._targetVertexId), + _fromCondition(nullptr), + _toCondition(nullptr) { + other.shortestPathCloneHelper(plan, *this, false); +} diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index df8a71c81730..31b473efcfb0 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -45,6 +45,13 @@ class ShortestPathNode : public GraphNode { friend class RedundantCalculationsReplacer; /// @brief constructor with a vocbase and a collection name + protected: + /// @brief Clone constructor, used for constructors of derived classes. + /// Does not clone recursively, does not clone properties (`other.plan()` is + /// expected to be the same as `plan)`, and does not register this node in the + /// plan. + ShortestPathNode(ExecutionPlan& plan, const ShortestPathNode& node); + public: ShortestPathNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, AstNode const* direction, AstNode const* start, AstNode const* target, @@ -124,6 +131,14 @@ class ShortestPathNode : public GraphNode { /// of blocks. void prepareOptions() override; + /// @brief Overrides GraphNode::Options with a more specific return type + /// (casts graph::BaseOptions* into graph::ShortestPathOptions*) + auto options() const -> graph::ShortestPathOptions*; + + private: + + void shortestPathCloneHelper(ExecutionPlan& plan, ShortestPathNode& c, bool withProperties) const; + private: /// @brief input variable only used if _vertexId is unused Variable const* _inStartVariable; diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index fdcc9ab0e416..3c6265775eb6 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -33,6 +33,7 @@ #include "Aql/ExecutionEngine.h" #include "Aql/ExecutionPlan.h" #include "Aql/Expression.h" +#include "Aql/GraphNode.h" #include "Aql/Query.h" #include "Aql/RegisterPlan.h" #include "Aql/SingleRowFetcher.h" @@ -785,10 +786,6 @@ void TraversalNode::getPruneVariables(std::vector& res) const { } } -bool TraversalNode::isEligibleAsSatelliteTraversal() const { - return graph() != nullptr && graph()->isSatellite(); -} - auto TraversalNode::options() const -> TraverserOptions* { #ifdef ARANGODB_ENABLE_MAINTAINER_MODE auto* opts = dynamic_cast(GraphNode::options()); diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index a9e8df9b5b13..a22963e56550 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -189,8 +189,6 @@ class TraversalNode : public GraphNode { bool allDirectionsEqual() const; - bool isEligibleAsSatelliteTraversal() const; - void getConditionVariables(std::vector&) const override; void getPruneVariables(std::vector&) const; From b23d31a6473a4ec714c55e718bfdefd58514620c Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 3 Mar 2020 18:18:18 +0100 Subject: [PATCH 24/47] optimized graph satellite ui --- .../js/templates/graphManagementView.html | 2 +- .../frontend/js/views/graphManagementView.js | 83 +++++++++++++------ 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html index 88b2f545d8d7..d6414cd58520 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/templates/graphManagementView.html @@ -49,7 +49,7 @@ <% graphs.forEach(function(graph) { var graphName = graph.get("_key"); var isSmart = graph.get("isSmart"); - var isSatellite = (graph.get("replicationFactor") == "satellite"); + var isSatellite = graph.get("isSatellite") || (graph.get("replicationFactor") == "satellite"); %>
diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js index 5c9907a77858..fff496bc1eb1 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js @@ -44,54 +44,84 @@ } if (id === 'smartGraph') { - // TODO: to be implemented properly - // add css class to entry - $('#createGraph').addClass('active'); // TODO: check addClass? - this.setCacheModeState(true); - this.showSmartGraphRows(); + this.setSmartGraphRows(true); } else if (id === 'satelliteGraph') { - // TODO: to be implemented properly - this.setCacheModeState(true); - this.hideSmartGraphRows(); + this.setSatelliteGraphRows(true); } else if (id === 'createGraph') { - // TODO: to be implemented properly - this.setCacheModeState(false); - this.hideSmartGraphRows(); - this.hideSatelliteGraphRows(); + this.setGeneralGraphRows(false); } }, - smartGraphRows: [ + // rows that are valid for general, smart & satellite + generalGraphRows: [ 'row_general-numberOfShards', 'row_general-replicationFactor', - 'row_general-writeConcern', + 'row_general-writeConcern' + ], + + // rows that needs to be added while creating smarties + neededSmartGraphRows: [ 'smartGraphInfo', + 'row_new-smartGraphAttribute', 'row_new-numberOfShards', 'row_new-replicationFactor', - 'row_new-writeConcern', - 'row_new-smartGraphAttribute' + 'row_new-writeConcern' + ], + + // rows that needs to be hidden while creating satellites + notNeededSatelliteGraphRows: [ + 'row_general-numberOfShards', + 'row_general-replicationFactor', + 'row_general-writeConcern' ], - hideSatelliteGraphRows: function () { - // TODO: to be implemented + setGeneralGraphRows: function (cache) { + this.setCacheModeState(cache); + this.hideSmartGraphRows(); + _.each(this.generalGraphRows, function (rowId) { + $('#' + rowId).show(); + }); + }, + + setSatelliteGraphRows: function (cache) { + $('#createGraph').addClass('active'); + this.setCacheModeState(cache); + + this.showGeneralGraphRows(); + this.hideSmartGraphRows(); + _.each(this.notNeededSatelliteGraphRows, function (rowId) { + $('#' + rowId).hide(); + }); }, - showSatelliteGraphRows: function () { - // TODO: to be implemented + setSmartGraphRows: function (cache) { + $('#createGraph').addClass('active'); + this.setCacheModeState(cache); + + this.hideGeneralGraphRows(); + _.each(this.neededSmartGraphRows, function (rowId) { + $('#' + rowId).show(); + }); }, hideSmartGraphRows: function () { - _.each(this.smartGraphRows, function (rowId) { + _.each(this.neededSmartGraphRows, function (rowId) { $('#' + rowId).hide(); }); }, - showSmartGraphRows: function () { - _.each(this.smartGraphRows, function (rowId) { + showGeneralGraphRows: function () { + _.each(this.generalGraphRows, function (rowId) { $('#' + rowId).show(); }); }, + hideGeneralGraphRows: function () { + _.each(this.generalGraphRows, function (rowId) { + $('#' + rowId).hide(); + }); + }, + redirectToGraphViewer: function (e) { var name = $(e.currentTarget).attr('id'); if (name) { @@ -715,6 +745,8 @@ minReplicationFactor: parseInt($('#new-writeConcern').val()), }; } + } else if ($('#tab-satelliteGraph').parent().hasClass('active')) { + newCollectionObject.replicationFactor = "satellite"; } else { if (frontendConfig.isCluster) { if ($('#general-numberOfShards').val().length > 0) { @@ -796,10 +828,11 @@ }); this.counter = 0; + // edit graph section if (graph) { if (isSmart) { title = 'Edit Smart Graph'; - } else if (isSatelite) { + } else if (isSatellite) { title = 'Edit Satellite Graph'; } else { title = 'Edit Graph'; @@ -876,6 +909,7 @@ window.modalView.createSuccessButton('Save', this.saveEditedGraph.bind(this)) ); } else { + // create graph section title = 'Create Graph'; tableContent.push( @@ -1107,6 +1141,7 @@ if ($('#tab-createGraph').parent().hasClass('active')) { // hide them by default, as we're showing general graph as default + // satellite does not need to appear here as it has no additional input fields self.hideSmartGraphRows(); } From 10ccf45d711cf04e4a6054e63720e8647db4ee9f Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 3 Mar 2020 18:18:39 +0100 Subject: [PATCH 25/47] static strings --- lib/Basics/StaticStrings.cpp | 1 + lib/Basics/StaticStrings.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lib/Basics/StaticStrings.cpp b/lib/Basics/StaticStrings.cpp index 509dbaf52595..44ba9844ba12 100644 --- a/lib/Basics/StaticStrings.cpp +++ b/lib/Basics/StaticStrings.cpp @@ -244,6 +244,7 @@ std::string const StaticStrings::Validators("validators"); // graph attribute names std::string const StaticStrings::GraphCollection("_graphs"); std::string const StaticStrings::GraphIsSmart("isSmart"); +std::string const StaticStrings::GraphIsSatellite("isSatellite"); std::string const StaticStrings::GraphFrom("from"); std::string const StaticStrings::GraphTo("to"); std::string const StaticStrings::GraphOptions("options"); diff --git a/lib/Basics/StaticStrings.h b/lib/Basics/StaticStrings.h index 069922cf30a0..4a59597ac83d 100644 --- a/lib/Basics/StaticStrings.h +++ b/lib/Basics/StaticStrings.h @@ -209,6 +209,7 @@ class StaticStrings { // collection attributes static std::string const NumberOfShards; static std::string const IsSmart; + static std::string const IsSatellite; static std::string const DistributeShardsLike; static std::string const CacheEnabled; static std::string const IndexBuckets; @@ -227,6 +228,7 @@ class StaticStrings { // graph attribute names static std::string const GraphCollection; static std::string const GraphIsSmart; + static std::string const GraphIsSatellite; static std::string const GraphFrom; static std::string const GraphTo; static std::string const GraphOptions; From f4e03b78b35dca25803e84267182d7706e50b61b Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 4 Mar 2020 18:05:51 +0100 Subject: [PATCH 26/47] properly add replication factor to options --- .../aardvark/APP/frontend/js/views/graphManagementView.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js index fff496bc1eb1..8f226b866fd6 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js @@ -746,7 +746,9 @@ }; } } else if ($('#tab-satelliteGraph').parent().hasClass('active')) { - newCollectionObject.replicationFactor = "satellite"; + newCollectionObject.options = { + replicationFactor: "satellite" + } } else { if (frontendConfig.isCluster) { if ($('#general-numberOfShards').val().length > 0) { From 4a5c8a0b3c31fc60b8088179a0e376a42c88a044 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 4 Mar 2020 18:08:16 +0100 Subject: [PATCH 27/47] rm wrong cast --- arangod/Aql/OptimizerRules.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 9fedce4c8775..3de2b361cb28 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -3640,7 +3640,6 @@ void arangodb::aql::createScatterGatherSnippet( SortElementVector const& elements, size_t const numberOfShards, std::unordered_map const& subqueries, Collection const* collection) { - collection = ExecutionNode::castTo(node)->collection(); // insert a scatter node auto* scatterNode = new ScatterNode(&plan, plan.nextId(), ScatterNode::ScatterType::SHARD); From de0d9f5a707ec8ae15fed5d4156f29dfc93faeca Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 6 Mar 2020 13:21:15 +0100 Subject: [PATCH 28/47] only persist cluster values when actually a cluster is used --- arangod/Graph/Graph.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/arangod/Graph/Graph.cpp b/arangod/Graph/Graph.cpp index e3a0db1f9d8a..8175dea192a0 100644 --- a/arangod/Graph/Graph.cpp +++ b/arangod/Graph/Graph.cpp @@ -313,15 +313,17 @@ void Graph::toPersistence(VPackBuilder& builder) const { builder.add(StaticStrings::KeyString, VPackValue(_graphName)); // Cluster Information - builder.add(StaticStrings::NumberOfShards, VPackValue(_numberOfShards)); - if (isSatellite()) { - builder.add(StaticStrings::ReplicationFactor, VPackValue(StaticStrings::Satellite)); - } else { - builder.add(StaticStrings::ReplicationFactor, VPackValue(_replicationFactor)); - builder.add(StaticStrings::MinReplicationFactor, VPackValue(_writeConcern)); // deprecated - builder.add(StaticStrings::WriteConcern, VPackValue(_writeConcern)); + if (arangodb::ServerState::instance()->isRunningInCluster()) { + builder.add(StaticStrings::NumberOfShards, VPackValue(_numberOfShards)); + if (isSatellite()) { + builder.add(StaticStrings::ReplicationFactor, VPackValue(StaticStrings::Satellite)); + } else { + builder.add(StaticStrings::ReplicationFactor, VPackValue(_replicationFactor)); + builder.add(StaticStrings::MinReplicationFactor, VPackValue(_writeConcern)); // deprecated + builder.add(StaticStrings::WriteConcern, VPackValue(_writeConcern)); + } + builder.add(StaticStrings::GraphIsSmart, VPackValue(isSmart())); } - builder.add(StaticStrings::GraphIsSmart, VPackValue(isSmart())); // EdgeDefinitions builder.add(VPackValue(StaticStrings::GraphEdgeDefinitions)); From db397ecb9bc1343d82a10a76eff8f6c018752c16 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 6 Mar 2020 14:22:58 +0100 Subject: [PATCH 29/47] prepare sat graph testsuite --- .../aql-graph-traversal-generic-graphs.js | 52 ++++++++++++++----- js/server/modules/@arangodb/general-graph.js | 1 - 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/js/server/modules/@arangodb/aql-graph-traversal-generic-graphs.js b/js/server/modules/@arangodb/aql-graph-traversal-generic-graphs.js index 326f4c279436..34f6090f4252 100644 --- a/js/server/modules/@arangodb/aql-graph-traversal-generic-graphs.js +++ b/js/server/modules/@arangodb/aql-graph-traversal-generic-graphs.js @@ -28,12 +28,14 @@ const tryRequire = (module) => { try { return require(module); - } catch(e) {} + } catch (e) { + } }; const internal = require("internal"); const db = internal.db; const sgm = tryRequire("@arangodb/smart-graph"); +const satgm = tryRequire("@arangodb/satellite-graph"); const cgm = require("@arangodb/general-graph"); const _ = require("lodash"); const assert = require("jsunity").jsUnity.assertions; @@ -43,10 +45,11 @@ const TestVariants = Object.freeze({ SingleServer: 1, GeneralGraph: 2, SmartGraph: 3, + SatelliteGraph: 4 }); class TestGraph { - constructor (graphName, edges, eRel, vn, en, protoSmartSharding, testVariant, numberOfShards) { + constructor(graphName, edges, eRel, vn, en, protoSmartSharding, testVariant, numberOfShards) { this.graphName = graphName; this.edges = edges; this.eRel = eRel; @@ -77,6 +80,13 @@ class TestGraph { sgm._create(this.name(), [this.eRel], [], options); break; } + case TestVariants.SatelliteGraph: { + const options = { + replicationFactor: 'satellite' + }; + satgm._create(this.name(), [this.eRel], [], options); + break; + } } if (this.testVariant === TestVariants.SingleServer) { @@ -144,12 +154,12 @@ class TestGraph { const done = () => ! _.values(exampleAttributeByShard) - .includes(null); + .includes(null); let i = 0; // const key = this.enterprise ? ProtoGraph.smartAttr() : "_key"; const key = "_key"; while (!done()) { - const value = this.enterprise ? i.toString() + ":" : i.toString() ; + const value = this.enterprise ? i.toString() + ":" : i.toString(); const doc = {[key]: value}; let shard; @@ -170,9 +180,11 @@ class TestGraph { } class ProtoGraph { - static smartAttr() { return "smart"; } + static smartAttr() { + return "smart"; + } - constructor (name, edges, generalShardings, smartShardings) { + constructor(name, edges, generalShardings, smartShardings) { this.protoGraphName = name; this.edges = edges; this.generalShardings = generalShardings; @@ -193,7 +205,7 @@ class ProtoGraph { } prepareGeneralGraphs() { - return this.generalShardings.map(numberOfShards => { + return this.generalShardings.map(numberOfShards => { const suffix = `_${numberOfShards}shards`; const vn = this.protoGraphName + '_Vertex' + suffix; const en = this.protoGraphName + '_Edge' + suffix; @@ -206,7 +218,7 @@ class ProtoGraph { } prepareSmartGraphs() { - return this.smartShardings.map((sharding, idx) => { + return this.smartShardings.map((sharding, idx) => { const {numberOfShards, vertexSharding} = sharding; const suffix = ProtoGraph._buildSmartSuffix(sharding, idx); @@ -220,6 +232,19 @@ class ProtoGraph { }); } + prepareSatelliteGraphs() { + // We're not able to test multiple shards in a Satellite Graph as a Satellite Graph has only one shard by default + const suffix = '_satellite'; + const numberOfShards = 1; + const vn = this.protoGraphName + '_Vertex' + suffix; + const en = this.protoGraphName + '_Edge' + suffix; + const gn = this.protoGraphName + '_Graph' + suffix; + + const eRel = cgm._relation(en, vn, vn); + + return [new TestGraph(gn, this.edges, eRel, vn, en, [], TestVariants.SatelliteGraph, numberOfShards)]; + } + static _buildSmartSuffix({numberOfShards, vertexSharding, name}, shardingIndex) { if (name) { return `_${name}`; @@ -231,7 +256,7 @@ class ProtoGraph { // "_2shards_s0-AB_s1-C" let suffix = `_${numberOfShards}shards_` + _.toPairs( - _.groupBy(vertexSharding, ([,s]) => s) + _.groupBy(vertexSharding, ([, s]) => s) ).map( ([s, vs]) => 's' + s + '-' + vs.map(([v,]) => v).join('') ).join('_'); @@ -694,9 +719,9 @@ protoGraphs.moreAdvancedPath = new ProtoGraph("moreAdvancedPath", [ assert.assertEqual('v510', vertices[vertices.length - 1]); const vi = (v) => Number(v.match(/^v(\d+)$/)[1]); - const vertexLevel = (v) => Math.floor(Math.log2(vi(v)+1)); + const vertexLevel = (v) => Math.floor(Math.log2(vi(v) + 1)); const parent = (v) => 'v' + parentIdx(vi(v)); - const ancestor = (v, i) => i === 0 ? v : ancestor(parent(v), i-1); + const ancestor = (v, i) => i === 0 ? v : ancestor(parent(v), i - 1); // when splitting the tree into perfect subtrees of depth 3 (this is // non-ambiguous), these helper functions return, for a given vertex, @@ -723,7 +748,7 @@ protoGraphs.moreAdvancedPath = new ProtoGraph("moreAdvancedPath", [ { // one shard per three levels name: "oneShardPerThreeLevels", numberOfShards: 3, - vertexSharding: vertices.map(v => [v, Math.floor(vertexLevel(v)/3)]), + vertexSharding: vertices.map(v => [v, Math.floor(vertexLevel(v) / 3)]), }, { // one shard per level name: "oneShardPerLevel", @@ -748,7 +773,8 @@ protoGraphs.moreAdvancedPath = new ProtoGraph("moreAdvancedPath", [ // ... name: "diagonalCutSharding", numberOfShards: 2, - vertexSharding: vertices.map(v => [v, [2,4,8,16,32,64,128,256].includes(vi(v)) ? 1 : 0]),}, + vertexSharding: vertices.map(v => [v, [2, 4, 8, 16, 32, 64, 128, 256].includes(vi(v)) ? 1 : 0]), + }, { // perfect subtrees of depth 3, each in different shards name: "perfectSubtreeSharding", numberOfShards: 73, diff --git a/js/server/modules/@arangodb/general-graph.js b/js/server/modules/@arangodb/general-graph.js index 7cc75676c884..15cda4302a9e 100644 --- a/js/server/modules/@arangodb/general-graph.js +++ b/js/server/modules/@arangodb/general-graph.js @@ -35,7 +35,6 @@ const GeneralGraph = internal.ArangoGeneralGraphModule; // This is supposed to be a class const ArangoGraph = internal.ArangoGraph; -//const arangodb = require("@arangodb"); // inherited graph class let CommonGraph = ggc.__GraphClass; From 2d27cd4f71fb1d66946f7b60e0539213d838dff7 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 6 Mar 2020 17:43:43 +0100 Subject: [PATCH 30/47] other isSmart --- arangod/Aql/GraphNode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index f8c04a02504e..c5955c4bd082 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -466,7 +466,7 @@ GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, _directions(other._directions), _options(std::move(options)), _optionsBuilt(false), - _isSmart(false) { + _isSmart(other.isSmart()) { setGraphInfoAndCopyColls(other.edgeColls(), other.vertexColls()); } From 10b4305d11f87e7975ef152bba91b2e591d29587 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 6 Mar 2020 17:47:51 +0100 Subject: [PATCH 31/47] move some logic we will use later to a helper --- js/common/modules/@arangodb/graph/helpers.js | 109 +++++++++++++++++ ...aql-traversal-restrict-edge-collections.js | 115 +----------------- 2 files changed, 113 insertions(+), 111 deletions(-) diff --git a/js/common/modules/@arangodb/graph/helpers.js b/js/common/modules/@arangodb/graph/helpers.js index 6f49b69faead..d1669e03a583 100644 --- a/js/common/modules/@arangodb/graph/helpers.js +++ b/js/common/modules/@arangodb/graph/helpers.js @@ -28,6 +28,7 @@ //////////////////////////////////////////////////////////////////////////////// var internal = require('internal'); +const db = require("@arangodb").db; function makeTree(k, depth, nrShards, subgraph) { // This creates a large graph (vertices and edges), which is a k-ary @@ -615,6 +616,113 @@ function storeGraph(r, Vname, Ename, Gname) { graph }; } +let runTraversalRestrictEdgeCollectionTests = function (vn, en, gn, checkOptimizerRule) { + let fillGraph = function () { + let keys = []; + let v = db._collection(vn); + for (let i = 0; i < 10; ++i) { + keys.push(v.insert({ value: i })._key); + } + + let e1 = db._collection(en + "1"); + e1.insert({ _from: vn + "/" + keys[0], _to: vn + "/" + keys[1] }); + e1.insert({ _from: vn + "/" + keys[1], _to: vn + "/" + keys[2] }); + e1.insert({ _from: vn + "/" + keys[2], _to: vn + "/" + keys[3] }); + e1.insert({ _from: vn + "/" + keys[3], _to: vn + "/" + keys[4] }); + e1.insert({ _from: vn + "/" + keys[4], _to: vn + "/" + keys[5] }); + + let e2 = db._collection(en + "2"); + e2.insert({ _from: vn + "/" + keys[0], _to: vn + "/" + keys[6] }); + e2.insert({ _from: vn + "/" + keys[1], _to: vn + "/" + keys[7] }); + e2.insert({ _from: vn + "/" + keys[7], _to: vn + "/" + keys[8] }); + e2.insert({ _from: vn + "/" + keys[8], _to: vn + "/" + keys[9] }); + + return keys; + }; + + let keys = fillGraph(); + + let queries = [ + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONs { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], + + [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], + [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], + [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], + [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], + ]; + + queries.forEach(function(q) { + if (checkOptimizerRule !== undefined) { + assertNotEqual(-1, AQL_EXPLAIN(q[0]).plan.rules.indexOf(checkOptimizerRule)); + } + const actual = db._query(q[0]).toArray(); + let expected = []; + q[1].forEach(function(e) { + expected.push(vn + "/" + keys[e]); + }); + assertEqual(actual, expected, q); + }); +}; + exports.makeTree = makeTree; exports.PoorMansRandom = PoorMansRandom; exports.makeClusteredGraph = makeClusteredGraph; @@ -623,3 +731,4 @@ exports.simulateDepthFirstSearch = simulateDepthFirstSearch; exports.checkBFSResult = checkBFSResult; exports.checkDFSResult = checkDFSResult; exports.storeGraph = storeGraph; +exports.runTraversalRestrictEdgeCollectionTests = runTraversalRestrictEdgeCollectionTests; diff --git a/tests/js/server/aql/aql-traversal-restrict-edge-collections.js b/tests/js/server/aql/aql-traversal-restrict-edge-collections.js index e82f95b54721..98c489909b45 100644 --- a/tests/js/server/aql/aql-traversal-restrict-edge-collections.js +++ b/tests/js/server/aql/aql-traversal-restrict-edge-collections.js @@ -29,8 +29,8 @@ //////////////////////////////////////////////////////////////////////////////// const jsunity = require("jsunity"); -const internal = require("internal"); const db = require("@arangodb").db; +const graphHelper = require("@arangodb/graph/helpers"); const isCoordinator = require('@arangodb/cluster').isCoordinator(); const isEnterprise = require("internal").isEnterprise(); @@ -39,113 +39,6 @@ function edgeCollectionRestrictionSuite() { const en = "UnitTestsEdges"; const gn = "UnitTestsGraph"; - let fillGraph = function () { - let keys = []; - let v = db._collection(vn); - for (let i = 0; i < 10; ++i) { - keys.push(v.insert({ value: i })._key); - } - - let e1 = db._collection(en + "1"); - e1.insert({ _from: vn + "/" + keys[0], _to: vn + "/" + keys[1] }); - e1.insert({ _from: vn + "/" + keys[1], _to: vn + "/" + keys[2] }); - e1.insert({ _from: vn + "/" + keys[2], _to: vn + "/" + keys[3] }); - e1.insert({ _from: vn + "/" + keys[3], _to: vn + "/" + keys[4] }); - e1.insert({ _from: vn + "/" + keys[4], _to: vn + "/" + keys[5] }); - - let e2 = db._collection(en + "2"); - e2.insert({ _from: vn + "/" + keys[0], _to: vn + "/" + keys[6] }); - e2.insert({ _from: vn + "/" + keys[1], _to: vn + "/" + keys[7] }); - e2.insert({ _from: vn + "/" + keys[7], _to: vn + "/" + keys[8] }); - e2.insert({ _from: vn + "/" + keys[8], _to: vn + "/" + keys[9] }); - - return keys; - }; - - let runTests = function (checkOptimizerRule) { - let keys = fillGraph(); - - let queries = [ - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1 OPTIONS { edgeCollections: "${en}1" } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONs { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}2 OPTIONS { edgeCollections: "${en}2" } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" ${en}1, ${en}2 OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 6 ] ], - - [ `WITH ${vn} FOR v IN 0..0 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0 ] ], - [ `WITH ${vn} FOR v IN 0..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 0, 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..1 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 6 ] ], - [ `WITH ${vn} FOR v IN 1..2 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 6, 7 ] ], - [ `WITH ${vn} FOR v IN 1..3 OUTBOUND "${vn}/${keys[0]}" GRAPH "${gn}" OPTIONS { edgeCollections: [ "${en}1", "${en}2" ] } SORT v._id RETURN DISTINCT v._id`, [ 1, 2, 3, 6, 7, 8 ] ], - ]; - - queries.forEach(function(q) { - if (checkOptimizerRule !== undefined) { - assertNotEqual(-1, AQL_EXPLAIN(q[0]).plan.rules.indexOf(checkOptimizerRule)); - } - const actual = db._query(q[0]).toArray(); - let expected = []; - q[1].forEach(function(e) { - expected.push(vn + "/" + keys[e]); - }); - assertEqual(actual, expected, q); - }); - }; - return { testBasic : function () { @@ -167,7 +60,7 @@ function edgeCollectionRestrictionSuite() { graphs._create(gn, [graphs._relation(en + "1", vn, vn), graphs._relation(en + "2", vn, vn)]); - runTests(); + graphHelper.runTraversalRestrictEdgeCollectionTests(vn, en, gn); } finally { cleanup(); } @@ -196,7 +89,7 @@ function edgeCollectionRestrictionSuite() { graphs._create(gn, [graphs._relation(en + "1", vn, vn), graphs._relation(en + "2", vn, vn)]); - runTests("cluster-one-shard"); + graphHelper.runTraversalRestrictEdgeCollectionTests(vn, en, gn,"cluster-one-shard"); } finally { cleanup(); } @@ -221,7 +114,7 @@ function edgeCollectionRestrictionSuite() { try { graphs._create(gn, [graphs._relation(en + "1", vn, vn), graphs._relation(en + "2", vn, vn)], null, { numberOfShards: 4, smartGraphAttribute: "aha" }); - runTests(); + graphHelper.runTraversalRestrictEdgeCollectionTests(vn, en, gn); } finally { cleanup(); } From 655b6972ed40b1c9e47cd8913e25ee4883a3c9b8 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 6 Mar 2020 17:55:38 +0100 Subject: [PATCH 32/47] remove log --- arangod/Sharding/ShardingInfo.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/arangod/Sharding/ShardingInfo.cpp b/arangod/Sharding/ShardingInfo.cpp index 0125ce8822c7..cf0acabb0bba 100644 --- a/arangod/Sharding/ShardingInfo.cpp +++ b/arangod/Sharding/ShardingInfo.cpp @@ -522,7 +522,6 @@ Result ShardingInfo::validateShardsAndReplicationFactor(arangodb::velocypack::Sl int64_t replicationFactorProbe = replicationFactorSlice.getNumber(); if (replicationFactorProbe == 0) { // TODO: Which configuration for satellites are valid regarding minRepl and writeConcern - LOG_DEVEL << "will create satellite collection - remove me after cleanup"; // valid for creating a satellite collection return Result(); } From cfe831d7d362e194ff9c9eb1184c2ad9722371b9 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Mon, 9 Mar 2020 21:40:03 +0100 Subject: [PATCH 33/47] sat check --- js/common/modules/@arangodb/general-graph-common.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js/common/modules/@arangodb/general-graph-common.js b/js/common/modules/@arangodb/general-graph-common.js index 184043cda881..d08facdc1356 100644 --- a/js/common/modules/@arangodb/general-graph-common.js +++ b/js/common/modules/@arangodb/general-graph-common.js @@ -1680,8 +1680,6 @@ exports._relation = function (relationName, fromVertexCollections, toVertexColle exports._graph = function (graphName) { let gdb = getGraphCollection(); let g; - let collections; - let orphanCollections; try { g = gdb.document(graphName); @@ -1700,6 +1698,12 @@ exports._graph = function (graphName) { err.errorMessage = 'The graph you requested is a SmartGraph (Enterprise Only)'; throw err; } + if (g.isSatellite) { + let err = new ArangoError(); + err.errorNum = arangodb.errors.ERROR_GRAPH_INVALID_GRAPH.code; + err.errorMessage = 'The graph you requested is a SatelliteGraph (Enterprise Only)'; + throw err; + } findOrCreateCollectionsByEdgeDefinitions(g.edgeDefinitions, true); return new Graph(g); From 3334ee3fb620315f1b97c785bec02101b0836678 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 10 Mar 2020 11:54:37 +0100 Subject: [PATCH 34/47] add isSatellite --- arangod/Graph/Graph.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/arangod/Graph/Graph.cpp b/arangod/Graph/Graph.cpp index 8175dea192a0..7858bf6c9c35 100644 --- a/arangod/Graph/Graph.cpp +++ b/arangod/Graph/Graph.cpp @@ -323,6 +323,7 @@ void Graph::toPersistence(VPackBuilder& builder) const { builder.add(StaticStrings::WriteConcern, VPackValue(_writeConcern)); } builder.add(StaticStrings::GraphIsSmart, VPackValue(isSmart())); + builder.add(StaticStrings::GraphIsSatellite, VPackValue(isSatellite())); } // EdgeDefinitions From d19763aafc930e1ed29a23a76d0f9767b35569f5 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Tue, 10 Mar 2020 11:56:08 +0100 Subject: [PATCH 35/47] do NOT set distributeShardsLike to empty in satellite collection case --- arangod/Sharding/ShardingInfo.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/arangod/Sharding/ShardingInfo.cpp b/arangod/Sharding/ShardingInfo.cpp index cf0acabb0bba..65545517ea6b 100644 --- a/arangod/Sharding/ShardingInfo.cpp +++ b/arangod/Sharding/ShardingInfo.cpp @@ -127,7 +127,6 @@ ShardingInfo::ShardingInfo(arangodb::velocypack::Slice info, LogicalCollection* _replicationFactor = 0; _writeConcern = 0; _numberOfShards = 1; - _distributeShardsLike = ""; _avoidServers.clear(); isError = false; isASatellite = true; From 6241f80ddcc06d218bb2e6505168303fecafc6c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 11 Mar 2020 12:41:00 +0100 Subject: [PATCH 36/47] Feature/satellite graphs kshortestpathnode (#11253) * Implemented KShortestPathsNode * Enabled SatelliteKShortestPathsNode * Renamed kShortestPathsCloneHelper properly --- arangod/Aql/GraphNode.cpp | 13 ++------- arangod/Aql/KShortestPathsNode.cpp | 47 ++++++++++++++++++++++++------ arangod/Aql/KShortestPathsNode.h | 15 ++++++++++ arangod/Aql/ShortestPathNode.cpp | 2 +- arangod/Aql/ShortestPathNode.h | 2 +- 5 files changed, 58 insertions(+), 21 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index c5955c4bd082..ca75e39c5e83 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -30,32 +30,27 @@ #include "ApplicationFeatures/ApplicationServer.h" #include "Aql/Ast.h" #include "Aql/Collection.h" -#include "Aql/ExecutionBlockImpl.h" #include "Aql/ExecutionEngine.h" #include "Aql/ExecutionPlan.h" #include "Aql/Expression.h" -#include "Aql/Query.h" #include "Aql/RegisterPlan.h" #include "Aql/SingleRowFetcher.h" #include "Aql/SortCondition.h" #include "Aql/TraversalExecutor.h" #include "Aql/Variable.h" -#include "Basics/StringUtils.h" #include "Basics/tryEmplaceHelper.h" #include "Cluster/ClusterFeature.h" #include "Cluster/ClusterTraverser.h" #include "Cluster/ServerState.h" -#include "Enterprise/Cluster/SmartGraphTraverser.h" #include "Graph/BaseOptions.h" #include "Graph/Graph.h" #include "Graph/SingleServerTraverser.h" #include "Graph/TraverserOptions.h" -#include "Indexes/Index.h" -#include "TraversalNode.h" #include "Utils/CollectionNameResolver.h" #include "VocBase/LogicalCollection.h" #include "VocBase/ticks.h" #ifdef USE_ENTERPRISE +#include "Enterprise/Aql/SatelliteKShortestPathsNode.h" #include "Enterprise/Aql/SatelliteShortestPathNode.h" #include "Enterprise/Aql/SatelliteTraversalNode.h" #endif @@ -718,10 +713,8 @@ bool GraphNode::isUsedAsSatellite() const { dynamic_cast(this); TRI_ASSERT((collectionAccessingNode != nullptr) == (nullptr != dynamic_cast(this) || - nullptr != dynamic_cast(this) - // TODO uncomment the next line when SatelliteKShortestPathsNode is implemented: - // || nullptr != dynamic_cast(this) - )); + nullptr != dynamic_cast(this) || + nullptr != dynamic_cast(this))); return collectionAccessingNode != nullptr && collectionAccessingNode->isUsedAsSatellite(); #endif diff --git a/arangod/Aql/KShortestPathsNode.cpp b/arangod/Aql/KShortestPathsNode.cpp index 8233320a86fb..f74504b46e3d 100644 --- a/arangod/Aql/KShortestPathsNode.cpp +++ b/arangod/Aql/KShortestPathsNode.cpp @@ -224,6 +224,18 @@ KShortestPathsNode::KShortestPathsNode(ExecutionPlan* plan, _toCondition = new AstNode(plan->getAst(), base.get("toCondition")); } +KShortestPathsNode::KShortestPathsNode(ExecutionPlan& plan, KShortestPathsNode const& other) + : GraphNode(plan, other, std::make_unique(*other.options())), + _pathOutVariable(other._pathOutVariable), + _inStartVariable(other._inStartVariable), + _startVertexId(other._startVertexId), + _inTargetVariable(other._inTargetVariable), + _targetVertexId(other._targetVertexId), + _fromCondition(nullptr), + _toCondition(nullptr) { + other.kShortestPathsCloneHelper(plan, *this, false); +} + void KShortestPathsNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags, std::unordered_set& seen) const { GraphNode::toVelocyPackHelper(nodes, flags, seen); // call base class method @@ -306,25 +318,32 @@ ExecutionNode* KShortestPathsNode::clone(ExecutionPlan* plan, bool withDependenc _directions, _inStartVariable, _startVertexId, _inTargetVariable, _targetVertexId, std::move(tmp)); + + kShortestPathsCloneHelper(*plan, *c, withProperties); + + return cloneHelper(std::move(c), withDependencies, withProperties); +} + +void KShortestPathsNode::kShortestPathsCloneHelper(ExecutionPlan& plan, + KShortestPathsNode& c, + bool withProperties) const { if (usesPathOutVariable()) { auto pathOutVariable = _pathOutVariable; if (withProperties) { - pathOutVariable = plan->getAst()->variables()->createVariable(pathOutVariable); + pathOutVariable = plan.getAst()->variables()->createVariable(pathOutVariable); } TRI_ASSERT(pathOutVariable != nullptr); - c->setPathOutput(pathOutVariable); + c.setPathOutput(pathOutVariable); } // Temporary Filter Objects - c->_tmpObjVariable = _tmpObjVariable; - c->_tmpObjVarNode = _tmpObjVarNode; - c->_tmpIdNode = _tmpIdNode; + c._tmpObjVariable = _tmpObjVariable; + c._tmpObjVarNode = _tmpObjVarNode; + c._tmpIdNode = _tmpIdNode; // Filter Condition Parts - c->_fromCondition = _fromCondition->clone(_plan->getAst()); - c->_toCondition = _toCondition->clone(_plan->getAst()); - - return cloneHelper(std::move(c), withDependencies, withProperties); + c._fromCondition = _fromCondition->clone(_plan->getAst()); + c._toCondition = _toCondition->clone(_plan->getAst()); } void KShortestPathsNode::prepareOptions() { @@ -369,3 +388,13 @@ void KShortestPathsNode::prepareOptions() { } _optionsBuilt = true; } + +auto KShortestPathsNode::options() const -> graph::ShortestPathOptions* { +#ifdef ARANGODB_ENABLE_MAINTAINER_MODE + auto* opts = dynamic_cast(GraphNode::options()); + TRI_ASSERT((GraphNode::options() == nullptr) == (opts == nullptr)); +#else + auto* opts = static_cast(GraphNode::options()); +#endif + return opts; +} diff --git a/arangod/Aql/KShortestPathsNode.h b/arangod/Aql/KShortestPathsNode.h index 0d7c793d5f3d..9ad96d754d42 100644 --- a/arangod/Aql/KShortestPathsNode.h +++ b/arangod/Aql/KShortestPathsNode.h @@ -46,6 +46,13 @@ class KShortestPathsNode : public GraphNode { friend class RedundantCalculationsReplacer; /// @brief constructor with a vocbase and a collection name + protected: + /// @brief Clone constructor, used for constructors of derived classes. + /// Does not clone recursively, does not clone properties (`other.plan()` is + /// expected to be the same as `plan)`, and does not register this node in the + /// plan. + KShortestPathsNode(ExecutionPlan& plan, KShortestPathsNode const& node); + public: KShortestPathsNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, AstNode const* direction, AstNode const* start, @@ -136,6 +143,14 @@ class KShortestPathsNode : public GraphNode { /// of blocks. void prepareOptions() override; + /// @brief Overrides GraphNode::Options with a more specific return type + /// (casts graph::BaseOptions* into graph::ShortestPathOptions*) + auto options() const -> graph::ShortestPathOptions*; + + private: + void kShortestPathsCloneHelper(ExecutionPlan& plan, KShortestPathsNode& c, + bool withProperties) const; + private: /// @brief path output variable Variable const* _pathOutVariable; diff --git a/arangod/Aql/ShortestPathNode.cpp b/arangod/Aql/ShortestPathNode.cpp index 47378e2325bd..92839da92efd 100644 --- a/arangod/Aql/ShortestPathNode.cpp +++ b/arangod/Aql/ShortestPathNode.cpp @@ -400,7 +400,7 @@ auto ShortestPathNode::options() const -> ShortestPathOptions* { return opts; } -ShortestPathNode::ShortestPathNode(ExecutionPlan& plan, const ShortestPathNode& other) +ShortestPathNode::ShortestPathNode(ExecutionPlan& plan, ShortestPathNode const& other) : GraphNode(plan, other, std::make_unique(*other.options())), _inStartVariable(other._inStartVariable), _startVertexId(other._startVertexId), diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index 058d2b9a8802..a379ecf337f6 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -50,7 +50,7 @@ class ShortestPathNode : public GraphNode { /// Does not clone recursively, does not clone properties (`other.plan()` is /// expected to be the same as `plan)`, and does not register this node in the /// plan. - ShortestPathNode(ExecutionPlan& plan, const ShortestPathNode& node); + ShortestPathNode(ExecutionPlan& plan, ShortestPathNode const& node); public: ShortestPathNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, From 49b646fc7d58336a5d234985cc33133be2bef1b6 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 11 Mar 2020 15:26:06 +0100 Subject: [PATCH 37/47] jslint --- .../aardvark/APP/frontend/js/views/graphManagementView.js | 2 +- js/common/modules/@arangodb/graph/helpers.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js index 8f226b866fd6..fd3dff0d0fab 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js @@ -748,7 +748,7 @@ } else if ($('#tab-satelliteGraph').parent().hasClass('active')) { newCollectionObject.options = { replicationFactor: "satellite" - } + }; } else { if (frontendConfig.isCluster) { if ($('#general-numberOfShards').val().length > 0) { diff --git a/js/common/modules/@arangodb/graph/helpers.js b/js/common/modules/@arangodb/graph/helpers.js index d1669e03a583..4d843695a5a7 100644 --- a/js/common/modules/@arangodb/graph/helpers.js +++ b/js/common/modules/@arangodb/graph/helpers.js @@ -1,4 +1,5 @@ /* jshint strict: true, unused: true */ +/* global AQL_EXPLAIN */ //////////////////////////////////////////////////////////////////////////////// /// @brief Helpers for graph tests @@ -27,8 +28,9 @@ /// @author Copyright 2016-2016, ArangoDB GmbH, Cologne, Germany //////////////////////////////////////////////////////////////////////////////// -var internal = require('internal'); +var jsunity = require('jsunity'); const db = require("@arangodb").db; +const {assertEqual, assertNotEqual} = jsunity.jsUnity.assertions; function makeTree(k, depth, nrShards, subgraph) { // This creates a large graph (vertices and edges), which is a k-ary From fcc8d151ac3b8022fb922e8687c8ce97f0de0866 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 11 Mar 2020 15:30:53 +0100 Subject: [PATCH 38/47] Implemented common ancestor SatelliteGraphNode --- arangod/Aql/GraphNode.cpp | 6 ++++++ arangod/Aql/GraphNode.h | 6 +++++- arangod/Aql/KShortestPathsNode.cpp | 4 +++- arangod/Aql/KShortestPathsNode.h | 2 +- arangod/Aql/OptimizerRules.cpp | 8 +++++++- arangod/Aql/ShortestPathNode.cpp | 4 +++- arangod/Aql/ShortestPathNode.h | 2 +- arangod/Aql/TraversalNode.cpp | 5 +++-- arangod/Aql/TraversalNode.h | 2 +- 9 files changed, 30 insertions(+), 9 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index ca75e39c5e83..fdd03d2a50a4 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -465,6 +465,12 @@ GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, setGraphInfoAndCopyColls(other.edgeColls(), other.vertexColls()); } +GraphNode::GraphNode(THIS_THROWS_WHEN_CALLED) + : ExecutionNode(nullptr, 0), _defaultDirection() { + TRI_ASSERT(false); + THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL); +} + std::string const& GraphNode::collectionToShardName(std::string const& collName) const { if (_collectionToShard.empty()) { return collName; diff --git a/arangod/Aql/GraphNode.h b/arangod/Aql/GraphNode.h index 12486d2b245e..7a058b7e60e9 100644 --- a/arangod/Aql/GraphNode.h +++ b/arangod/Aql/GraphNode.h @@ -56,13 +56,14 @@ namespace aql { // * Smart Graph Handling class GraphNode : public ExecutionNode { - public: + protected: /// @brief constructor with a vocbase and a collection name GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, AstNode const* direction, AstNode const* graph, std::unique_ptr options); GraphNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& base); + public: bool isUsedAsSatellite() const; bool isEligibleAsSatelliteTraversal() const; @@ -82,6 +83,9 @@ class GraphNode : public ExecutionNode { GraphNode(ExecutionPlan& plan, GraphNode const& other, std::unique_ptr options); + struct THIS_THROWS_WHEN_CALLED{}; + explicit GraphNode(THIS_THROWS_WHEN_CALLED); + std::string const& collectionToShardName(std::string const& collName) const; public: diff --git a/arangod/Aql/KShortestPathsNode.cpp b/arangod/Aql/KShortestPathsNode.cpp index f74504b46e3d..665ac26cd32e 100644 --- a/arangod/Aql/KShortestPathsNode.cpp +++ b/arangod/Aql/KShortestPathsNode.cpp @@ -224,8 +224,10 @@ KShortestPathsNode::KShortestPathsNode(ExecutionPlan* plan, _toCondition = new AstNode(plan->getAst(), base.get("toCondition")); } +// This constructor is only used from SatelliteTraversalNode, and GraphNode +// is virtually inherited; thus its constructor is never called from here. KShortestPathsNode::KShortestPathsNode(ExecutionPlan& plan, KShortestPathsNode const& other) - : GraphNode(plan, other, std::make_unique(*other.options())), + : GraphNode(GraphNode::THIS_THROWS_WHEN_CALLED{}), _pathOutVariable(other._pathOutVariable), _inStartVariable(other._inStartVariable), _startVertexId(other._startVertexId), diff --git a/arangod/Aql/KShortestPathsNode.h b/arangod/Aql/KShortestPathsNode.h index 9ad96d754d42..665dff1ad54a 100644 --- a/arangod/Aql/KShortestPathsNode.h +++ b/arangod/Aql/KShortestPathsNode.h @@ -41,7 +41,7 @@ struct ShortestPathOptions; namespace aql { /// @brief class KShortestPathsNode -class KShortestPathsNode : public GraphNode { +class KShortestPathsNode : public virtual GraphNode { friend class ExecutionBlock; friend class RedundantCalculationsReplacer; diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 3de2b361cb28..9ed0e3a4e2d7 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2034,7 +2034,13 @@ class arangodb::aql::RedundantCalculationsReplacer final template void replaceStartTargetVariables(ExecutionNode* en) { - auto node = static_cast(en); + auto node = std::invoke([en](auto) { + if constexpr (std::is_base_of_v) { + return dynamic_cast(en); + } else { + return static_cast(en); + } + }, 0); if (node->_inStartVariable != nullptr) { node->_inStartVariable = Variable::replace(node->_inStartVariable, _replacements); } diff --git a/arangod/Aql/ShortestPathNode.cpp b/arangod/Aql/ShortestPathNode.cpp index 92839da92efd..6736b442ecff 100644 --- a/arangod/Aql/ShortestPathNode.cpp +++ b/arangod/Aql/ShortestPathNode.cpp @@ -400,8 +400,10 @@ auto ShortestPathNode::options() const -> ShortestPathOptions* { return opts; } +// This constructor is only used from SatelliteTraversalNode, and GraphNode +// is virtually inherited; thus its constructor is never called from here. ShortestPathNode::ShortestPathNode(ExecutionPlan& plan, ShortestPathNode const& other) - : GraphNode(plan, other, std::make_unique(*other.options())), + : GraphNode(GraphNode::THIS_THROWS_WHEN_CALLED{}), _inStartVariable(other._inStartVariable), _startVertexId(other._startVertexId), _inTargetVariable(other._inTargetVariable), diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index a379ecf337f6..7d3a7764a9d4 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -40,7 +40,7 @@ struct ShortestPathOptions; namespace aql { /// @brief class ShortestPathNode -class ShortestPathNode : public GraphNode { +class ShortestPathNode : public virtual GraphNode { friend class ExecutionBlock; friend class RedundantCalculationsReplacer; diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index d46983c2ee86..fb252d33a359 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -276,9 +276,10 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co #endif } +// This constructor is only used from SatelliteTraversalNode, and GraphNode +// is virtually inherited; thus its constructor is never called from here. TraversalNode::TraversalNode(ExecutionPlan& plan, TraversalNode const& other) - : GraphNode(plan, other, - std::make_unique(*other.options())), + : GraphNode(GraphNode::THIS_THROWS_WHEN_CALLED{}), _pathOutVariable(nullptr), _inVariable(other._inVariable), _vertexId(other._vertexId), diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index a22963e56550..f42cce64c4eb 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -46,7 +46,7 @@ struct TraverserOptions; namespace aql { /// @brief class TraversalNode -class TraversalNode : public GraphNode { +class TraversalNode : public virtual GraphNode { class TraversalEdgeConditionBuilder final : public EdgeConditionBuilder { private: /// @brief reference to the outer traversal node From fc32950452b088261d2556521188d7572f6274ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Wed, 11 Mar 2020 17:52:09 +0100 Subject: [PATCH 39/47] Make castTo work for GraphNodes again --- arangod/Aql/ExecutionNode.h | 9 ++++++++- lib/Basics/TypeTraits.h | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 lib/Basics/TypeTraits.h diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h index 565c926c11d7..455068ca3986 100644 --- a/arangod/Aql/ExecutionNode.h +++ b/arangod/Aql/ExecutionNode.h @@ -68,6 +68,7 @@ #include "Aql/WalkerWorker.h" #include "Aql/types.h" #include "Basics/Common.h" +#include "Basics/TypeTraits.h" #include "Containers/HashSet.h" namespace arangodb { @@ -203,7 +204,13 @@ class ExecutionNode { TRI_ASSERT(result != nullptr); return result; #else - return static_cast(node); + // At least GraphNode is virtually inherited by its subclasses. We have to + // use dynamic_cast for these types. + if constexpr (can_static_cast_v) { + return static_cast(node); + } else { + return dynamic_cast(node); + } #endif } diff --git a/lib/Basics/TypeTraits.h b/lib/Basics/TypeTraits.h new file mode 100644 index 000000000000..53d58b131854 --- /dev/null +++ b/lib/Basics/TypeTraits.h @@ -0,0 +1,40 @@ +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2020 ArangoDB GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is ArangoDB GmbH, Cologne, Germany +/// +/// @author Tobias Gödderz +//////////////////////////////////////////////////////////////////////////////// + +#ifndef ARANGODB_BASICS_TYPETRAITS_H +#define ARANGODB_BASICS_TYPETRAITS_H + +namespace arangodb { + +template +struct can_static_cast : std::false_type {}; + +template +struct can_static_cast((Base*)nullptr))>> + : std::true_type {}; + +template +constexpr bool can_static_cast_v = can_static_cast::value; + +} // namespace arangodb + +#endif // ARANGODB_BASICS_TYPETRAITS_H From ae50a9f78d38fd8caf8d5315287589ce7a7760a7 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 11 Mar 2020 18:00:06 +0100 Subject: [PATCH 40/47] fixed a failing test --- tests/js/client/http/api-gharial-spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/js/client/http/api-gharial-spec.js b/tests/js/client/http/api-gharial-spec.js index 9cda0a0ee2ca..e1046cd5f855 100644 --- a/tests/js/client/http/api-gharial-spec.js +++ b/tests/js/client/http/api-gharial-spec.js @@ -104,6 +104,7 @@ describe('_api/gharial', () => { minReplicationFactor: Joi.number().integer().min(1).required(), writeConcern: Joi.number().integer().min(1).required(), isSmart: Joi.boolean().required(), + isSatellite: Joi.boolean().required(), orphanCollections: Joi.array().items(Joi.string()).required(), edgeDefinitions: Joi.array().items(edgeDefinition).required() }); From 7be51fec7cf4e7645a03f1fbb74df006709bad8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 12 Mar 2020 10:23:28 +0100 Subject: [PATCH 41/47] Added SatelliteGraphNode::graph() (#11256) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Gödderz --- arangod/Aql/KShortestPathsNode.h | 2 +- arangod/Aql/ShortestPathNode.h | 2 +- arangod/Aql/TraversalNode.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arangod/Aql/KShortestPathsNode.h b/arangod/Aql/KShortestPathsNode.h index 665dff1ad54a..fddcd295c285 100644 --- a/arangod/Aql/KShortestPathsNode.h +++ b/arangod/Aql/KShortestPathsNode.h @@ -143,7 +143,7 @@ class KShortestPathsNode : public virtual GraphNode { /// of blocks. void prepareOptions() override; - /// @brief Overrides GraphNode::Options with a more specific return type + /// @brief Overrides GraphNode::options() with a more specific return type /// (casts graph::BaseOptions* into graph::ShortestPathOptions*) auto options() const -> graph::ShortestPathOptions*; diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index 7d3a7764a9d4..77336c805b67 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -131,7 +131,7 @@ class ShortestPathNode : public virtual GraphNode { /// of blocks. void prepareOptions() override; - /// @brief Overrides GraphNode::Options with a more specific return type + /// @brief Overrides GraphNode::options() with a more specific return type /// (casts graph::BaseOptions* into graph::ShortestPathOptions*) auto options() const -> graph::ShortestPathOptions*; diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index f42cce64c4eb..c33b3c40dd20 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -202,7 +202,7 @@ class TraversalNode : public virtual GraphNode { // You are not responsible for it! Expression* pruneExpression() const { return _pruneExpression.get(); } - /// @brief Overrides GraphNode::Options with a more specific return type + /// @brief Overrides GraphNode::options() with a more specific return type /// (casts graph::BaseOptions* into traverser::TraverserOptions*) auto options() const -> traverser::TraverserOptions*; From f85153b55686d6426b0e83dc469df19fc902090d Mon Sep 17 00:00:00 2001 From: hkernbach Date: Thu, 12 Mar 2020 13:40:28 +0100 Subject: [PATCH 42/47] fix http gharial test --- tests/js/client/http/api-gharial-spec.js | 41 ++++++++++++++++-------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/tests/js/client/http/api-gharial-spec.js b/tests/js/client/http/api-gharial-spec.js index e1046cd5f855..769a14724ae0 100644 --- a/tests/js/client/http/api-gharial-spec.js +++ b/tests/js/client/http/api-gharial-spec.js @@ -38,6 +38,7 @@ const arango = arangodb.arango; const ERRORS = arangodb.errors; const db = arangodb.db; const internal = require('internal'); +const isCluster = internal.isCluster(); const wait = internal.wait; const url = '/_api/gharial'; @@ -94,20 +95,32 @@ describe('_api/gharial', () => { from: Joi.array().items(Joi.string()).required(), to: Joi.array().items(Joi.string()).required() }); - const schema = Joi.object({ - "_key": Joi.string().required(), - "_rev": Joi.string().required(), - "_id": Joi.string().required(), - name: Joi.string().required(), - numberOfShards: Joi.number().integer().min(1).required(), - replicationFactor: Joi.number().integer().min(1).required(), - minReplicationFactor: Joi.number().integer().min(1).required(), - writeConcern: Joi.number().integer().min(1).required(), - isSmart: Joi.boolean().required(), - isSatellite: Joi.boolean().required(), - orphanCollections: Joi.array().items(Joi.string()).required(), - edgeDefinitions: Joi.array().items(edgeDefinition).required() - }); + let schema; + if (isCluster) { + schema = Joi.object({ + "_key": Joi.string().required(), + "_rev": Joi.string().required(), + "_id": Joi.string().required(), + name: Joi.string().required(), + numberOfShards: Joi.number().integer().min(1).required(), + replicationFactor: Joi.number().integer().min(1).required(), + minReplicationFactor: Joi.number().integer().min(1).required(), + writeConcern: Joi.number().integer().min(1).required(), + isSmart: Joi.boolean().required(), + isSatellite: Joi.boolean().required(), + orphanCollections: Joi.array().items(Joi.string()).required(), + edgeDefinitions: Joi.array().items(edgeDefinition).required() + }); + } else { + schema = Joi.object({ + "_key": Joi.string().required(), + "_rev": Joi.string().required(), + "_id": Joi.string().required(), + name: Joi.string().required(), + orphanCollections: Joi.array().items(Joi.string()).required(), + edgeDefinitions: Joi.array().items(edgeDefinition).required() + }); + } const res = schema.validate(graph); expect(res.error).to.be.null; }; From 5251d2d26547c0f530c7ebe139589edb6fa9220f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 12 Mar 2020 19:28:43 +0100 Subject: [PATCH 43/47] Added exception check for replicationFactor == 0; made isSatellite more explicit --- arangod/Graph/Graph.cpp | 30 ++++++++++++++++++++---------- arangod/Graph/Graph.h | 4 +++- lib/Basics/StaticStrings.h | 1 - 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/arangod/Graph/Graph.cpp b/arangod/Graph/Graph.cpp index 7858bf6c9c35..0c4011b6d170 100644 --- a/arangod/Graph/Graph.cpp +++ b/arangod/Graph/Graph.cpp @@ -92,7 +92,9 @@ Graph::Graph(velocypack::Slice const& slice) _replicationFactor( Helper::getNumericValue(slice, StaticStrings::ReplicationFactor, 1)), _writeConcern(::getWriteConcern(slice)), - _rev(Helper::getStringValue(slice, StaticStrings::RevString, "")) { + _rev(Helper::getStringValue(slice, StaticStrings::RevString, "")), + _isSatellite(Helper::getStringRef(slice, StaticStrings::ReplicationFactor, + velocypack::StringRef("")) == StaticStrings::Satellite) { // If this happens we have a document without an _key Attribute. if (_graphName.empty()) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, @@ -116,9 +118,11 @@ Graph::Graph(velocypack::Slice const& slice) if (slice.hasKey(StaticStrings::GraphOrphans)) { insertOrphanCollections(slice.get(StaticStrings::GraphOrphans)); } - if (slice.hasKey(StaticStrings::ReplicationFactor) && - slice.get(StaticStrings::ReplicationFactor).isString() && - slice.get(StaticStrings::ReplicationFactor).isEqualString(StaticStrings::Satellite)) { + TRI_ASSERT((slice.hasKey(StaticStrings::ReplicationFactor) && + slice.get(StaticStrings::ReplicationFactor).isString() && + slice.get(StaticStrings::ReplicationFactor).isEqualString(StaticStrings::Satellite)) == + _isSatellite); + if (_isSatellite) { setReplicationFactor(0); } } @@ -146,12 +150,18 @@ Graph::Graph(std::string&& graphName, VPackSlice const& info, VPackSlice const& if (options.isObject()) { _numberOfShards = Helper::getNumericValue(options, StaticStrings::NumberOfShards, 1); - if (Helper::getStringValue(options.get(StaticStrings::ReplicationFactor), - "1") == StaticStrings::Satellite) { + if (Helper::getStringRef(options.get(StaticStrings::ReplicationFactor), + velocypack::StringRef("")) == StaticStrings::Satellite) { + _isSatellite = true; setReplicationFactor(0); } else { _replicationFactor = Helper::getNumericValue(options, StaticStrings::ReplicationFactor, 1); + if (_replicationFactor < 1) { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, + StaticStrings::ReplicationFactor + + " must be greater than zero"); + } _writeConcern = ::getWriteConcern(options); } } @@ -698,10 +708,7 @@ void Graph::verticesToVpack(VPackBuilder& builder) const { bool Graph::isSmart() const { return false; } bool Graph::isSatellite() const { - if (replicationFactor() == 0) { - return true; - } - return false; + return _isSatellite; } void Graph::createCollectionOptions(VPackBuilder& builder, bool waitForSync) const { @@ -713,6 +720,9 @@ void Graph::createCollectionOptions(VPackBuilder& builder, bool waitForSync) con if (!isSatellite()) { builder.add(StaticStrings::MinReplicationFactor, VPackValue(writeConcern())); // deprecated builder.add(StaticStrings::WriteConcern, VPackValue(writeConcern())); + TRI_ASSERT(replicationFactor() > 0); + } else { + TRI_ASSERT(replicationFactor() == 0); } builder.add(StaticStrings::ReplicationFactor, VPackValue(replicationFactor())); diff --git a/arangod/Graph/Graph.h b/arangod/Graph/Graph.h index eea9050a079a..04237bb03281 100644 --- a/arangod/Graph/Graph.h +++ b/arangod/Graph/Graph.h @@ -288,7 +288,6 @@ class Graph { uint64_t _numberOfShards; /// @brief replication factor of this graph - /// if the value is set to 0, it will be threaten as a satellite graph uint64_t _replicationFactor; /// @brief write concern for this graph @@ -296,6 +295,9 @@ class Graph { /// @brief revision of this graph std::string _rev; + + /// @brief whether this graph is a satellite graph + bool _isSatellite = false; }; // helper functions diff --git a/lib/Basics/StaticStrings.h b/lib/Basics/StaticStrings.h index 34b2596314dd..f8a443eaa951 100644 --- a/lib/Basics/StaticStrings.h +++ b/lib/Basics/StaticStrings.h @@ -210,7 +210,6 @@ class StaticStrings { // collection attributes static std::string const NumberOfShards; static std::string const IsSmart; - static std::string const IsSatellite; static std::string const IsSmartChild; static std::string const DistributeShardsLike; static std::string const CacheEnabled; From 43e047be80f61b0d1adcf7a5bc60f9667c6a5f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Fri, 13 Mar 2020 11:28:10 +0100 Subject: [PATCH 44/47] Make OneShard not set distributeShardsLike for satellite collections --- arangod/RestHandler/RestReplicationHandler.cpp | 10 ++++++++-- arangod/VocBase/Methods/Collections.cpp | 10 +++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/arangod/RestHandler/RestReplicationHandler.cpp b/arangod/RestHandler/RestReplicationHandler.cpp index 8fb7ffe7d850..e1d4bdfe5e56 100644 --- a/arangod/RestHandler/RestReplicationHandler.cpp +++ b/arangod/RestHandler/RestReplicationHandler.cpp @@ -1219,11 +1219,17 @@ Result RestReplicationHandler::processRestoreCollectionCoordinator( toMerge.add("id", VPackValue(newId)); if (_vocbase.server().getFeature().forceOneShard()) { + auto const isSatellite = + VelocyPackHelper::getStringRef(parameters, StaticStrings::ReplicationFactor, + velocypack::StringRef{""}) == StaticStrings::Satellite; + // force one shard, and force distributeShardsLike to be "_graphs" toMerge.add(StaticStrings::NumberOfShards, VPackValue(1)); - if (!_vocbase.IsSystemName(name)) { + if (!_vocbase.IsSystemName(name) && !isSatellite) { // system-collections will be sharded normally. only user collections will - // get the forced sharding + // get the forced sharding. + // satellite collections must not be sharded like a non-satellite + // collection. toMerge.add(StaticStrings::DistributeShardsLike, VPackValue(_vocbase.shardingPrototypeName())); } diff --git a/arangod/VocBase/Methods/Collections.cpp b/arangod/VocBase/Methods/Collections.cpp index 6e21177232ca..c5d059e530e9 100644 --- a/arangod/VocBase/Methods/Collections.cpp +++ b/arangod/VocBase/Methods/Collections.cpp @@ -316,9 +316,17 @@ Result Collections::create(TRI_vocbase_t& vocbase, // system-collections will be sharded normally. only user collections will get // the forced sharding if (vocbase.server().getFeature().forceOneShard()) { + auto const isSatellite = + Helper::getStringRef(info.properties, StaticStrings::ReplicationFactor, + velocypack::StringRef{""}) == StaticStrings::Satellite; // force one shard, and force distributeShardsLike to be "_graphs" helper.add(StaticStrings::NumberOfShards, VPackValue(1)); - helper.add(StaticStrings::DistributeShardsLike, VPackValue(vocbase.shardingPrototypeName())); + if (!isSatellite) { + // satellite collections must not be sharded like a non-satellite + // collection. + helper.add(StaticStrings::DistributeShardsLike, + VPackValue(vocbase.shardingPrototypeName())); + } } else if (vocbase.sharding() == "single" && numberOfShards <= 1) { auto distributeSlice = info.properties.get(StaticStrings::DistributeShardsLike); if (distributeSlice.isNone()) { From 996defe94f571d07d2daa0ea73710675c22cd3ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Fri, 13 Mar 2020 15:28:26 +0100 Subject: [PATCH 45/47] Bugfix when removing multiple satellite joins --- arangod/Aql/EngineInfoContainerDBServerServerBased.cpp | 6 +++--- arangod/Aql/EngineInfoContainerDBServerServerBased.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp index 7c5c5909d606..836e41867bb7 100644 --- a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp +++ b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp @@ -194,7 +194,7 @@ EngineInfoContainerDBServerServerBased::EngineInfoContainerDBServerServerBased(Q // NOTE: We need to start with _lastSnippetID > 0. 0 is reserved for GraphNodes } -void EngineInfoContainerDBServerServerBased::injectVertexColletions(GraphNode* graphNode) { +void EngineInfoContainerDBServerServerBased::injectVertexCollections(GraphNode* graphNode) { auto const& vCols = graphNode->vertexColls(); if (vCols.empty()) { std::map const* allCollections = @@ -228,7 +228,7 @@ void EngineInfoContainerDBServerServerBased::addNode(ExecutionNode* node) { case ExecutionNode::K_SHORTEST_PATHS: { auto* const graphNode = ExecutionNode::castTo(node); graphNode->prepareOptions(); - injectVertexColletions(graphNode); + injectVertexCollections(graphNode); break; } default: @@ -517,7 +517,7 @@ void EngineInfoContainerDBServerServerBased::cleanupEngines( // the DBServers. The GraphNode itself will retain on the coordinator. void EngineInfoContainerDBServerServerBased::addGraphNode(GraphNode* node) { node->prepareOptions(); - injectVertexColletions(node); + injectVertexCollections(node); // SnippetID does not matter on GraphNodes _shardLocking.addNode(node, 0); _graphNodes.emplace_back(node); diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.h b/arangod/Aql/EngineInfoContainerDBServerServerBased.h index 1a5666bbafef..f6ddb664eef0 100644 --- a/arangod/Aql/EngineInfoContainerDBServerServerBased.h +++ b/arangod/Aql/EngineInfoContainerDBServerServerBased.h @@ -173,7 +173,7 @@ class EngineInfoContainerDBServerServerBased { ServerID const& server, std::string const& serverDest, std::vector const& didCreateEngine) const; - void injectVertexColletions(GraphNode* node); + void injectVertexCollections(GraphNode* node); private: std::stack, std::vector>> _snippetStack; From 1611c4babd01da7d67b5a579174d9e1cf9b25ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Sat, 14 Mar 2020 13:58:12 +0100 Subject: [PATCH 46/47] Fixed clone() for all SatelliteNodes --- arangod/Aql/GraphNode.cpp | 5 +++-- arangod/Aql/KShortestPathsNode.h | 2 +- arangod/Aql/QuerySnippet.cpp | 4 +++- arangod/Aql/ShortestPathNode.h | 2 +- arangod/Aql/TraversalNode.cpp | 16 +++++++++++----- arangod/Aql/TraversalNode.h | 8 ++++++-- arangod/Graph/BaseOptions.cpp | 8 +++++--- arangod/Graph/BaseOptions.h | 5 ++++- arangod/Graph/Graph.cpp | 4 ++++ arangod/Graph/Graph.h | 5 +++++ arangod/Graph/ShortestPathOptions.cpp | 14 ++++++++++++++ arangod/Graph/ShortestPathOptions.h | 9 ++++++++- arangod/Graph/TraverserOptions.cpp | 17 ++++++++++------- arangod/Graph/TraverserOptions.h | 5 ++++- 14 files changed, 79 insertions(+), 25 deletions(-) diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index fdd03d2a50a4..73fdf61eed61 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -453,7 +453,7 @@ GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, _vocbase(other._vocbase), _vertexOutVariable(nullptr), _edgeOutVariable(nullptr), - _graphObj(nullptr), + _graphObj(other.graph()), _tmpObjVariable(_plan->getAst()->variables()->createTemporaryVariable()), _tmpObjVarNode(_plan->getAst()->createNodeReference(_tmpObjVariable)), _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), @@ -461,7 +461,8 @@ GraphNode::GraphNode(ExecutionPlan& plan, GraphNode const& other, _directions(other._directions), _options(std::move(options)), _optionsBuilt(false), - _isSmart(other.isSmart()) { + _isSmart(other.isSmart()), + _collectionToShard(other._collectionToShard) { setGraphInfoAndCopyColls(other.edgeColls(), other.vertexColls()); } diff --git a/arangod/Aql/KShortestPathsNode.h b/arangod/Aql/KShortestPathsNode.h index fddcd295c285..cff6ba956a13 100644 --- a/arangod/Aql/KShortestPathsNode.h +++ b/arangod/Aql/KShortestPathsNode.h @@ -88,7 +88,7 @@ class KShortestPathsNode : public virtual GraphNode { /// @brief clone ExecutionNode recursively ExecutionNode* clone(ExecutionPlan* plan, bool withDependencies, - bool withProperties) const override final; + bool withProperties) const override; bool usesPathOutVariable() const { return _pathOutVariable != nullptr; } Variable const& pathOutVariable() const { diff --git a/arangod/Aql/QuerySnippet.cpp b/arangod/Aql/QuerySnippet.cpp index b264592f1822..5e819ddecac3 100644 --- a/arangod/Aql/QuerySnippet.cpp +++ b/arangod/Aql/QuerySnippet.cpp @@ -362,6 +362,8 @@ ResultT>> QuerySnippet::pre } } } else { + TRI_ASSERT(graphNode->isUsedAsSatellite()); + TRI_ASSERT(USE_ENTERPRISE); for (auto* aqlCollection : graphNode->collections()) { auto const& shards = shardLocking.shardsForSnippet(id(), aqlCollection); for (auto const& shard : shards) { @@ -391,7 +393,7 @@ ResultT>> QuerySnippet::pre std::set myExp; auto modNode = dynamic_cast(exp.node); - // Only accessing nodes can endup here. + // Only accessing nodes can end up here. TRI_ASSERT(modNode != nullptr); auto col = modNode->collection(); // Should be hit earlier, a modification node here is required to have a collection diff --git a/arangod/Aql/ShortestPathNode.h b/arangod/Aql/ShortestPathNode.h index 77336c805b67..0b7570cfc606 100644 --- a/arangod/Aql/ShortestPathNode.h +++ b/arangod/Aql/ShortestPathNode.h @@ -86,7 +86,7 @@ class ShortestPathNode : public virtual GraphNode { /// @brief clone ExecutionNode recursively ExecutionNode* clone(ExecutionPlan* plan, bool withDependencies, - bool withProperties) const override final; + bool withProperties) const override; /// @brief Test if this node uses an in variable or constant for start bool usesStartInVariable() const { return _inStartVariable != nullptr; } diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index fb252d33a359..a4812f83bb57 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -278,14 +278,17 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co // This constructor is only used from SatelliteTraversalNode, and GraphNode // is virtually inherited; thus its constructor is never called from here. -TraversalNode::TraversalNode(ExecutionPlan& plan, TraversalNode const& other) +TraversalNode::TraversalNode(ExecutionPlan& plan, TraversalNode const& other, + bool const allowAlreadyBuiltCopy) : GraphNode(GraphNode::THIS_THROWS_WHEN_CALLED{}), _pathOutVariable(nullptr), _inVariable(other._inVariable), _vertexId(other._vertexId), _fromCondition(nullptr), _toCondition(nullptr) { - TRI_ASSERT(!other._optionsBuilt); + if (!allowAlreadyBuiltCopy) { + TRI_ASSERT(!other._optionsBuilt); + } other.traversalCloneHelper(plan, *this, false); } @@ -541,15 +544,18 @@ std::unique_ptr TraversalNode::createBlock( /// @brief clone ExecutionNode recursively ExecutionNode* TraversalNode::clone(ExecutionPlan* plan, bool withDependencies, bool withProperties) const { - TRI_ASSERT(!_optionsBuilt); - auto oldOpts = options(); - std::unique_ptr tmp = std::make_unique(*oldOpts); + auto* oldOpts = options(); + std::unique_ptr tmp = std::make_unique(*oldOpts, true); auto c = std::make_unique(plan, _id, _vocbase, _edgeColls, _vertexColls, _inVariable, _vertexId, _defaultDirection, _directions, std::move(tmp)); traversalCloneHelper(*plan, *c, withProperties); + if (_optionsBuilt) { + c->prepareOptions(); + } + return cloneHelper(std::move(c), withDependencies, withProperties); } diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index c33b3c40dd20..a27aeab64f37 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -99,7 +99,11 @@ class TraversalNode : public virtual GraphNode { /// Does not clone recursively, does not clone properties (`other.plan()` is /// expected to be the same as `plan)`, and does not register this node in the /// plan. - TraversalNode(ExecutionPlan& plan, TraversalNode const& other); + /// When allowAlreadyBuiltCopy is true, allows copying a node which options + /// are already prepared. prepareOptions() has to be called on the copy if + /// the options were already prepared on other (_optionsBuilt)! + TraversalNode(ExecutionPlan& plan, TraversalNode const& other, + bool allowAlreadyBuiltCopy = false); public: /// @brief return the type of the node @@ -116,7 +120,7 @@ class TraversalNode : public virtual GraphNode { /// @brief clone ExecutionNode recursively ExecutionNode* clone(ExecutionPlan* plan, bool withDependencies, - bool withProperties) const override final; + bool withProperties) const override; /// @brief Test if this node uses an in variable or constant bool usesInVariable() const { return _inVariable != nullptr; } diff --git a/arangod/Graph/BaseOptions.cpp b/arangod/Graph/BaseOptions.cpp index cda7ea40bd20..f759faf21620 100644 --- a/arangod/Graph/BaseOptions.cpp +++ b/arangod/Graph/BaseOptions.cpp @@ -175,15 +175,17 @@ BaseOptions::BaseOptions(arangodb::aql::Query* query) _isCoordinator(arangodb::ServerState::instance()->isCoordinator()), _tmpVar(nullptr) {} -BaseOptions::BaseOptions(BaseOptions const& other) +BaseOptions::BaseOptions(BaseOptions const& other, bool const allowAlreadyBuiltCopy) : _query(other._query), _ctx(_query), _trx(other._trx), _produceVertices(other._produceVertices), _isCoordinator(arangodb::ServerState::instance()->isCoordinator()), _tmpVar(nullptr) { - TRI_ASSERT(other._baseLookupInfos.empty()); - TRI_ASSERT(other._tmpVar == nullptr); + if (!allowAlreadyBuiltCopy) { + TRI_ASSERT(other._baseLookupInfos.empty()); + TRI_ASSERT(other._tmpVar == nullptr); + } } BaseOptions::BaseOptions(arangodb::aql::Query* query, VPackSlice info, VPackSlice collections) diff --git a/arangod/Graph/BaseOptions.h b/arangod/Graph/BaseOptions.h index 2817ca86a834..eca73b9ffdee 100644 --- a/arangod/Graph/BaseOptions.h +++ b/arangod/Graph/BaseOptions.h @@ -91,7 +91,10 @@ struct BaseOptions { /// @brief This copy constructor is only working during planning phase. /// After planning this node should not be copied anywhere. - explicit BaseOptions(BaseOptions const&); + /// When allowAlreadyBuiltCopy is true, the constructor also works after + /// the planning phase; however, the options have to be prepared again + /// (see GraphNode::prepareOptions() and its overrides) + BaseOptions(BaseOptions const&, bool allowAlreadyBuiltCopy = false); BaseOptions& operator=(BaseOptions const&) = delete; BaseOptions(arangodb::aql::Query*, arangodb::velocypack::Slice, arangodb::velocypack::Slice); diff --git a/arangod/Graph/Graph.cpp b/arangod/Graph/Graph.cpp index 0c4011b6d170..b2033e834841 100644 --- a/arangod/Graph/Graph.cpp +++ b/arangod/Graph/Graph.cpp @@ -167,6 +167,10 @@ Graph::Graph(std::string&& graphName, VPackSlice const& info, VPackSlice const& } } +auto Graph::clone() const -> std::unique_ptr { + return std::make_unique>(*this); +} + void Graph::parseEdgeDefinitions(VPackSlice edgeDefs) { if (!edgeDefs.isArray()) { THROW_ARANGO_EXCEPTION_MESSAGE( diff --git a/arangod/Graph/Graph.h b/arangod/Graph/Graph.h index 04237bb03281..a609602a67eb 100644 --- a/arangod/Graph/Graph.h +++ b/arangod/Graph/Graph.h @@ -133,6 +133,11 @@ class Graph { Graph(std::string&& graphName, velocypack::Slice const& info, velocypack::Slice const& options); + /** + * @brief virtual copy constructor + */ + virtual auto clone() const -> std::unique_ptr; + public: virtual ~Graph() = default; diff --git a/arangod/Graph/ShortestPathOptions.cpp b/arangod/Graph/ShortestPathOptions.cpp index 86d672d07ab0..8048fc545f1c 100644 --- a/arangod/Graph/ShortestPathOptions.cpp +++ b/arangod/Graph/ShortestPathOptions.cpp @@ -216,3 +216,17 @@ void ShortestPathOptions::isQueryKilledCallback() const { THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } } + +ShortestPathOptions::ShortestPathOptions(ShortestPathOptions const& other, + bool const allowAlreadyBuiltCopy) + : BaseOptions(other, allowAlreadyBuiltCopy), + start{other.start}, + direction{other.direction}, + weightAttribute{other.weightAttribute}, + defaultWeight{other.defaultWeight}, + bidirectional{other.bidirectional}, + multiThreaded{other.multiThreaded}, + end{other.end}, + startBuilder{other.startBuilder}, + endBuilder{other.endBuilder}, + _reverseLookupInfos{other._reverseLookupInfos} {} diff --git a/arangod/Graph/ShortestPathOptions.h b/arangod/Graph/ShortestPathOptions.h index b8c4dbc391d7..46c0bffbb7c5 100644 --- a/arangod/Graph/ShortestPathOptions.h +++ b/arangod/Graph/ShortestPathOptions.h @@ -60,7 +60,14 @@ struct ShortestPathOptions : public BaseOptions { // @brief DBServer-constructor used by TraverserEngines ShortestPathOptions(aql::Query* query, arangodb::velocypack::Slice info, arangodb::velocypack::Slice collections); - ~ShortestPathOptions(); + ~ShortestPathOptions() override; + + /// @brief This copy constructor is only working during planning phase. + /// After planning this node should not be copied anywhere. + /// When allowAlreadyBuiltCopy is true, the constructor also works after + /// the planning phase; however, the options have to be prepared again + /// (see ShortestPathNode / KShortestPathsNode ::prepareOptions()) + ShortestPathOptions(ShortestPathOptions const& other, bool allowAlreadyBuiltCopy = false); // Creates a complete Object containing all EngineInfo // in the given builder. diff --git a/arangod/Graph/TraverserOptions.cpp b/arangod/Graph/TraverserOptions.cpp index 9a38d5060b12..10ab0bda4804 100644 --- a/arangod/Graph/TraverserOptions.cpp +++ b/arangod/Graph/TraverserOptions.cpp @@ -364,8 +364,9 @@ arangodb::traverser::TraverserOptions::TraverserOptions(arangodb::aql::Query* qu _produceVertices = VPackHelper::getBooleanValue(info, "produceVertices", true); } -arangodb::traverser::TraverserOptions::TraverserOptions(TraverserOptions const& other) - : BaseOptions(static_cast(other)), +arangodb::traverser::TraverserOptions::TraverserOptions(TraverserOptions const& other, + bool const allowAlreadyBuiltCopy) + : BaseOptions(static_cast(other), allowAlreadyBuiltCopy), _baseVertexExpression(nullptr), _traverser(nullptr), minDepth(other.minDepth), @@ -376,11 +377,13 @@ arangodb::traverser::TraverserOptions::TraverserOptions(TraverserOptions const& uniqueEdges(other.uniqueEdges), vertexCollections(other.vertexCollections), edgeCollections(other.edgeCollections) { - TRI_ASSERT(other._baseLookupInfos.empty()); - TRI_ASSERT(other._depthLookupInfo.empty()); - TRI_ASSERT(other._vertexExpressions.empty()); - TRI_ASSERT(other._tmpVar == nullptr); - TRI_ASSERT(other._baseVertexExpression == nullptr); + if (!allowAlreadyBuiltCopy) { + TRI_ASSERT(other._baseLookupInfos.empty()); + TRI_ASSERT(other._depthLookupInfo.empty()); + TRI_ASSERT(other._vertexExpressions.empty()); + TRI_ASSERT(other._tmpVar == nullptr); + TRI_ASSERT(other._baseVertexExpression == nullptr); + } // Check for illegal option combination: TRI_ASSERT(uniqueEdges != TraverserOptions::UniquenessLevel::GLOBAL); diff --git a/arangod/Graph/TraverserOptions.h b/arangod/Graph/TraverserOptions.h index b9a5538c7224..afbdfaa10d07 100644 --- a/arangod/Graph/TraverserOptions.h +++ b/arangod/Graph/TraverserOptions.h @@ -103,7 +103,10 @@ struct TraverserOptions : public graph::BaseOptions { /// @brief This copy constructor is only working during planning phase. /// After planning this node should not be copied anywhere. - TraverserOptions(TraverserOptions const&); + /// When allowAlreadyBuiltCopy is true, the constructor also works after + /// the planning phase; however, the options have to be prepared again + /// (see TraversalNode::prepareOptions()) + TraverserOptions(TraverserOptions const& other, bool allowAlreadyBuiltCopy = false); TraverserOptions& operator=(TraverserOptions const&) = delete; virtual ~TraverserOptions(); From 59f36532c1a3852705632e7cb56cf83754e9468a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Sat, 14 Mar 2020 15:07:37 +0100 Subject: [PATCH 47/47] Fixed non-enterprise compile error --- arangod/Aql/QuerySnippet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arangod/Aql/QuerySnippet.cpp b/arangod/Aql/QuerySnippet.cpp index 5e819ddecac3..0212e6d9b39b 100644 --- a/arangod/Aql/QuerySnippet.cpp +++ b/arangod/Aql/QuerySnippet.cpp @@ -363,7 +363,9 @@ ResultT>> QuerySnippet::pre } } else { TRI_ASSERT(graphNode->isUsedAsSatellite()); - TRI_ASSERT(USE_ENTERPRISE); +#ifndef USE_ENTERPRISE + TRI_ASSERT(false); +#endif for (auto* aqlCollection : graphNode->collections()) { auto const& shards = shardLocking.shardsForSnippet(id(), aqlCollection); for (auto const& shard : shards) {