Let the optimizer keep a single instance in Query Engine (#314)

* Let the optimizer keep a single instance in Query Engine

* Fix failed test case

* Fix typo

resources/gflags.json

@@ -9,8 +9,7 @@
         "wal_ttl",
         "enable_reservoir_sampling",
         "custom_filter_interval_secs",
-        "enable_multi_versions",
-        "enable_optimizer"
+        "enable_multi_versions"
     ],
     "NESTED": [
         "rocksdb_db_options",

src/executor/algo/BFSShortestPathExecutor.h

@@ -4,8 +4,8 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
-#ifndef EXECUTOR_QUERY_BFSSHORTESTPATHEXECUTOR_H_
-#define EXECUTOR_QUERY_BFSSHORTESTPATHEXECUTOR_H_
+#ifndef EXECUTOR_ALGO_BFSSHORTESTPATHEXECUTOR_H_
+#define EXECUTOR_ALGO_BFSSHORTESTPATHEXECUTOR_H_
 
 #include "executor/Executor.h"
 
@@ -23,4 +23,4 @@ private:
 };
 }  // namespace graph
 }  // namespace nebula
-#endif
+#endif   // EXECUTOR_ALGO_BFSSHORTESTPATHEXECUTOR_H_

src/executor/algo/ConjunctPathExecutor.h

@@ -4,8 +4,8 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
-#ifndef EXECUTOR_QUERY_CONJUNCTPATHEXECUTOR_H_
-#define EXECUTOR_QUERY_CONJUNCTPATHEXECUTOR_H_
+#ifndef EXECUTOR_ALGO_CONJUNCTPATHEXECUTOR_H_
+#define EXECUTOR_ALGO_CONJUNCTPATHEXECUTOR_H_
 
 #include "executor/Executor.h"
 
@@ -38,4 +38,4 @@ private:
 };
 }  // namespace graph
 }  // namespace nebula
-#endif  // EXECUTOR_QUERY_CONJUNCTPATHEXECUTOR_H_
+#endif  // EXECUTOR_ALGO_CONJUNCTPATHEXECUTOR_H_

src/optimizer/OptGroup.cpp

@@ -94,7 +94,7 @@ Status OptGroup::explore(const OptRule *rule) {
     return Status::OK();
 }
 
-Status OptGroup::exploreUtilMaxRound(const OptRule *rule) {
+Status OptGroup::exploreUntilMaxRound(const OptRule *rule) {
     auto maxRound = kMaxExplorationRound;
     while (!isExplored(rule)) {
         if (0 < maxRound--) {
@@ -148,12 +148,12 @@ Status OptGroupExpr::explore(const OptRule *rule) {
 
     for (auto dep : dependencies_) {
         DCHECK(dep != nullptr);
-        NG_RETURN_IF_ERROR(dep->exploreUtilMaxRound(rule));
+        NG_RETURN_IF_ERROR(dep->exploreUntilMaxRound(rule));
     }
 
     for (auto body : bodies_) {
         DCHECK(body != nullptr);
-        NG_RETURN_IF_ERROR(body->exploreUtilMaxRound(rule));
+        NG_RETURN_IF_ERROR(body->exploreUntilMaxRound(rule));
     }
     return Status::OK();
 }

src/optimizer/OptGroup.h

@@ -50,7 +50,7 @@ public:
     }
 
     Status explore(const OptRule *rule);
-    Status exploreUtilMaxRound(const OptRule *rule);
+    Status exploreUntilMaxRound(const OptRule *rule);
     double getCost() const;
     const graph::PlanNode *getPlan() const;
 

src/optimizer/Optimizer.cpp

@@ -23,42 +23,44 @@ using nebula::graph::SingleDependencyNode;
 namespace nebula {
 namespace opt {
 
-Optimizer::Optimizer(QueryContext *qctx, std::vector<const RuleSet *> ruleSets)
-    : qctx_(qctx), ruleSets_(std::move(ruleSets)) {
+Optimizer::Optimizer(std::vector<const RuleSet *> ruleSets) : ruleSets_(std::move(ruleSets)) {}
+
+StatusOr<const PlanNode *> Optimizer::findBestPlan(QueryContext *qctx) {
     DCHECK(qctx != nullptr);
-}
 
-StatusOr<const PlanNode *> Optimizer::findBestPlan(PlanNode *root) {
-    DCHECK(root != nullptr);
-    rootNode_ = root;
-    NG_RETURN_IF_ERROR(prepare());
-    NG_RETURN_IF_ERROR(doExploration());
-    return rootGroup_->getPlan();
+    auto root = qctx->plan()->root();
+    auto status = prepare(qctx, root);
+    NG_RETURN_IF_ERROR(status);
+    auto rootGroup = std::move(status).value();
+
+    NG_RETURN_IF_ERROR(doExploration(rootGroup));
+    return rootGroup->getPlan();
 }
 
-Status Optimizer::prepare() {
-    visitedNodes_.clear();
-    rootGroup_ = convertToGroup(rootNode_);
-    return Status::OK();
+StatusOr<OptGroup *> Optimizer::prepare(QueryContext *qctx, PlanNode *root) {
+    std::unordered_map<int64_t, OptGroup *> visited;
+    return convertToGroup(qctx, root, &visited);
 }
 
-Status Optimizer::doExploration() {
+Status Optimizer::doExploration(OptGroup *rootGroup) {
     for (auto ruleSet : ruleSets_) {
         for (auto rule : ruleSet->rules()) {
-            NG_RETURN_IF_ERROR(rootGroup_->exploreUtilMaxRound(rule));
+            NG_RETURN_IF_ERROR(rootGroup->exploreUntilMaxRound(rule));
         }
     }
     return Status::OK();
 }
 
-OptGroup *Optimizer::convertToGroup(PlanNode *node) {
-    auto iter = visitedNodes_.find(node->id());
-    if (iter != visitedNodes_.cend()) {
+OptGroup *Optimizer::convertToGroup(QueryContext *qctx,
+                                    PlanNode *node,
+                                    std::unordered_map<int64_t, OptGroup *> *visited) {
+    auto iter = visited->find(node->id());
+    if (iter != visited->cend()) {
         return iter->second;
     }
 
-    auto group = OptGroup::create(qctx_);
-    auto groupExpr = group->makeGroupExpr(qctx_, node);
+    auto group = OptGroup::create(qctx);
+    auto groupExpr = group->makeGroupExpr(qctx, node);
 
     switch (node->dependencies().size()) {
         case 0: {
@@ -68,26 +70,29 @@ OptGroup *Optimizer::convertToGroup(PlanNode *node) {
         case 1: {
             if (node->kind() == PlanNode::Kind::kSelect) {
                 auto select = static_cast<Select *>(node);
-                auto then = convertToGroup(const_cast<PlanNode *>(select->then()));
+                auto then = convertToGroup(qctx, const_cast<PlanNode *>(select->then()), visited);
                 groupExpr->addBody(then);
-                auto otherwise = convertToGroup(const_cast<PlanNode *>(select->otherwise()));
+                auto otherNode = const_cast<PlanNode *>(select->otherwise());
+                auto otherwise = convertToGroup(qctx, otherNode, visited);
                 groupExpr->addBody(otherwise);
             } else if (node->kind() == PlanNode::Kind::kLoop) {
                 auto loop = static_cast<Loop *>(node);
-                auto body = convertToGroup(const_cast<PlanNode *>(loop->body()));
+                auto body = convertToGroup(qctx, const_cast<PlanNode *>(loop->body()), visited);
                 groupExpr->addBody(body);
             }
             auto dep = static_cast<SingleDependencyNode *>(node)->dep();
             DCHECK(dep != nullptr);
-            auto depGroup = convertToGroup(const_cast<graph::PlanNode *>(dep));
+            auto depGroup = convertToGroup(qctx, const_cast<graph::PlanNode *>(dep), visited);
             groupExpr->dependsOn(depGroup);
             break;
         }
         case 2: {
             auto bNode = static_cast<BiInputNode *>(node);
-            auto leftGroup = convertToGroup(const_cast<graph::PlanNode *>(bNode->left()));
+            auto leftNode = const_cast<graph::PlanNode *>(bNode->left());
+            auto leftGroup = convertToGroup(qctx, leftNode, visited);
             groupExpr->dependsOn(leftGroup);
-            auto rightGroup = convertToGroup(const_cast<graph::PlanNode *>(bNode->right()));
+            auto rightNode = const_cast<graph::PlanNode *>(bNode->right());
+            auto rightGroup = convertToGroup(qctx, rightNode, visited);
             groupExpr->dependsOn(rightGroup);
             break;
         }
@@ -97,7 +102,7 @@ OptGroup *Optimizer::convertToGroup(PlanNode *node) {
             break;
         }
     }
-    visitedNodes_.emplace(node->id(), group);
+    visited->emplace(node->id(), group);
     return group;
 }
 

src/optimizer/Optimizer.h

@@ -25,22 +25,20 @@ class RuleSet;
 
 class Optimizer final {
 public:
-    Optimizer(graph::QueryContext *qctx, std::vector<const RuleSet *> ruleSets);
+    explicit Optimizer(std::vector<const RuleSet *> ruleSets);
     ~Optimizer() = default;
 
-    StatusOr<const graph::PlanNode *> findBestPlan(graph::PlanNode *root);
+    StatusOr<const graph::PlanNode *> findBestPlan(graph::QueryContext *qctx);
 
 private:
-    Status prepare();
-    Status doExploration();
+    StatusOr<OptGroup *> prepare(graph::QueryContext *qctx, graph::PlanNode *root);
+    Status doExploration(OptGroup *rootGroup);
 
-    OptGroup *convertToGroup(graph::PlanNode *node);
+    OptGroup *convertToGroup(graph::QueryContext *qctx,
+                             graph::PlanNode *node,
+                             std::unordered_map<int64_t, OptGroup *> *visited);
 
-    graph::QueryContext *qctx_{nullptr};
-    graph::PlanNode *rootNode_{nullptr};
-    OptGroup *rootGroup_{nullptr};
     std::vector<const RuleSet *> ruleSets_;
-    std::unordered_map<int64_t, OptGroup *> visitedNodes_;
 };
 
 }   // namespace opt

src/service/QueryEngine.cpp

@@ -4,25 +4,20 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
-#include "common/base/Base.h"
 #include "service/QueryEngine.h"
-#include "service/QueryInstance.h"
+
+#include "common/base/Base.h"
 #include "context/QueryContext.h"
+#include "optimizer/OptRule.h"
+#include "service/QueryInstance.h"
 
 DECLARE_bool(local_config);
+DECLARE_bool(enable_optimizer);
 DECLARE_string(meta_server_addrs);
 
 namespace nebula {
 namespace graph {
 
-QueryEngine::QueryEngine() {
-}
-
-
-QueryEngine::~QueryEngine() {
-}
-
-
 Status QueryEngine::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor) {
     auto addrs = network::NetworkUtils::toHosts(FLAGS_meta_server_addrs);
     if (!addrs.ok()) {
@@ -52,6 +47,12 @@ Status QueryEngine::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor
                                                              metaClient_.get());
     charsetInfo_ = CharsetInfo::instance();
 
+    std::vector<const opt::RuleSet*> rulesets{&opt::RuleSet::DefaultRules()};
+    if (FLAGS_enable_optimizer) {
+        rulesets.emplace_back(&opt::RuleSet::QueryRules());
+    }
+    optimizer_ = std::make_unique<opt::Optimizer>(rulesets);
+
     return Status::OK();
 }
 
@@ -62,7 +63,7 @@ void QueryEngine::execute(RequestContextPtr rctx) {
                                                storage_.get(),
                                                metaClient_.get(),
                                                charsetInfo_);
-    auto* instance = new QueryInstance(std::move(ectx));
+    auto* instance = new QueryInstance(std::move(ectx), optimizer_.get());
     instance->execute();
 }
 

src/service/QueryEngine.h

@@ -18,6 +18,7 @@
 #include "common/clients/storage/GraphStorageClient.h"
 #include "common/network/NetworkUtils.h"
 #include "common/charset/Charset.h"
+#include "optimizer/Optimizer.h"
 #include <folly/executors/IOThreadPoolExecutor.h>
 
 /**
@@ -31,8 +32,8 @@ namespace graph {
 
 class QueryEngine final : public cpp::NonCopyable, public cpp::NonMovable {
 public:
-    QueryEngine();
-    ~QueryEngine();
+    QueryEngine() = default;
+    ~QueryEngine() = default;
 
     Status init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExecutor);
 
@@ -49,6 +50,7 @@ private:
     // std::unique_ptr<meta::ClientBasedGflagsManager>   gflagsManager_;
     std::unique_ptr<storage::GraphStorageClient>      storage_;
     std::unique_ptr<meta::MetaClient>                 metaClient_;
+    std::unique_ptr<opt::Optimizer>                   optimizer_;
     CharsetInfo*                                      charsetInfo_{nullptr};
 };
 

src/service/QueryInstance.cpp

@@ -14,7 +14,6 @@
 #include "planner/ExecutionPlan.h"
 #include "planner/PlanNode.h"
 #include "scheduler/Scheduler.h"
-#include "service/GraphFlags.h"
 #include "validator/Validator.h"
 
 using nebula::opt::Optimizer;
@@ -24,14 +23,10 @@ using nebula::opt::RuleSet;
 namespace nebula {
 namespace graph {
 
-QueryInstance::QueryInstance(std::unique_ptr<QueryContext> qctx) {
+QueryInstance::QueryInstance(std::unique_ptr<QueryContext> qctx, Optimizer *optimizer) {
     qctx_ = std::move(qctx);
+    optimizer_ = DCHECK_NOTNULL(optimizer);
     scheduler_ = std::make_unique<Scheduler>(qctx_.get());
-    std::vector<const RuleSet *> rulesets{&RuleSet::DefaultRules()};
-    if (FLAGS_enable_optimizer) {
-        rulesets.emplace_back(&RuleSet::QueryRules());
-    }
-    optimizer_ = std::make_unique<Optimizer>(qctx_.get(), std::move(rulesets));
 }
 
 void QueryInstance::execute() {
@@ -67,7 +62,7 @@ Status QueryInstance::validateAndOptimize() {
 
     NG_RETURN_IF_ERROR(Validator::validate(sentence_.get(), qctx()));
 
-    auto rootStatus = optimizer_->findBestPlan(qctx_->plan()->root());
+    auto rootStatus = optimizer_->findBestPlan(qctx_.get());
     NG_RETURN_IF_ERROR(rootStatus);
     auto newRoot = std::move(rootStatus).value();
     qctx_->setPlan(std::make_unique<ExecutionPlan>(const_cast<PlanNode *>(newRoot)));

src/service/QueryInstance.h

@@ -26,7 +26,7 @@ namespace graph {
 
 class QueryInstance final : public cpp::NonCopyable, public cpp::NonMovable {
 public:
-    explicit QueryInstance(std::unique_ptr<QueryContext> qctx);
+    explicit QueryInstance(std::unique_ptr<QueryContext> qctx, opt::Optimizer* optimizer);
     ~QueryInstance() = default;
 
     void execute();
@@ -56,7 +56,7 @@ private:
     std::unique_ptr<Sentence>                   sentence_;
     std::unique_ptr<QueryContext>               qctx_;
     std::unique_ptr<Scheduler>                  scheduler_;
-    std::unique_ptr<opt::Optimizer>             optimizer_;
+    opt::Optimizer*                             optimizer_{nullptr};
 };
 
 }   // namespace graph

tests/admin/test_configs.py

@@ -59,7 +59,6 @@ class TestConfigs(NebulaTestSuite):
         expected_result = [['GRAPH', 'v', 'int', 'MUTABLE', v],
                            ['GRAPH', 'minloglevel', 'int', 'MUTABLE', 0],
                            ['GRAPH', 'slow_op_threshhold_ms', 'int', 'MUTABLE', 50],
-                           ['GRAPH', 'enable_optimizer', 'bool', 'MUTABLE', True],
                            ['GRAPH', 'heartbeat_interval_secs', 'int', 'MUTABLE', 1],
                            ['GRAPH', 'meta_client_retry_times', 'int', 'MUTABLE', 3]]
         self.check_out_of_order_result(resp, expected_result)