cherry pick: 0208 (#3856)

* skip heartbeat when role in meta client is not set (#3855) * Try to improve the performance of appendVertices (#3857) * Try to improve the performance of appendVertices * fix a unstable tck * keep consistent with 2.6.x (#3859) confirm with sophie.xie Co-authored-by: Doodle <[email protected]> Co-authored-by: jie.wang <[email protected]> Co-authored-by: Harris.Chu <[email protected]>
vesoft-inc · Feb 9, 2022 · 96db138 · 96db138
1 parent 9cec50c
commit 96db138
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 27 deletions.
diff --git a/.github/workflows/rc.yml b/.github/workflows/rc.yml
@@ -40,7 +40,7 @@ jobs:
       - uses: ./.github/actions/tagname-action
         id: tag
       - name: package
-        run: ./package/package.sh -v ${{ steps.tag.outputs.tag }} -t RelWithDebInfo -r OFF -p ON -s TRUE
+        run: ./package/package.sh -v ${{ steps.tag.outputs.tagnum }} -t RelWithDebInfo -r OFF -p ON -s TRUE
       - name: output some vars
         run: |
           tar zcf ${{ env.CPACK_DIR }}/nebula-${{ steps.tag.outputs.tagnum }}.tar.gz --exclude=${{ env.BUILD_DIR }} ./*

diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp
@@ -94,15 +94,19 @@ MetaClient::~MetaClient() {
 }
 
 bool MetaClient::isMetadReady() {
-  auto ret = heartbeat().get();
-  if (!ret.ok()) {
-    LOG(ERROR) << "Heartbeat failed, status:" << ret.status();
-    return ready_;
-  } else if (options_.role_ == cpp2::HostRole::STORAGE &&
-             metaServerVersion_ != EXPECT_META_VERSION) {
-    LOG(ERROR) << "Expect meta version is " << EXPECT_META_VERSION << ", but actual is "
-               << metaServerVersion_;
-    return ready_;
+  // UNKNOWN is reserved for tools such as upgrader, in that case the ip/port is not set. We do
+  // not send heartbeat to meta to avoid writing error host info (e.g. Host("", 0))
+  if (options_.role_ != cpp2::HostRole::UNKNOWN) {
+    auto ret = heartbeat().get();
+    if (!ret.ok()) {
+      LOG(ERROR) << "Heartbeat failed, status:" << ret.status();
+      return ready_;
+    } else if (options_.role_ == cpp2::HostRole::STORAGE &&
+               metaServerVersion_ != EXPECT_META_VERSION) {
+      LOG(ERROR) << "Expect meta version is " << EXPECT_META_VERSION << ", but actual is "
+                 << metaServerVersion_;
+      return ready_;
+    }
   }
 
   // ready_ will be set in loadData

diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h
@@ -185,7 +185,6 @@ struct MetaClientOptions {
   MetaClientOptions(const MetaClientOptions& opt)
       : localHost_(opt.localHost_),
         clusterId_(opt.clusterId_.load()),
-        inStoraged_(opt.inStoraged_),
         serviceName_(opt.serviceName_),
         skipConfig_(opt.skipConfig_),
         role_(opt.role_),
@@ -197,13 +196,12 @@ struct MetaClientOptions {
   HostAddr localHost_{"", 0};
   // Current cluster Id, it is required by storaged only.
   std::atomic<ClusterID> clusterId_{0};
-  // If current client being used in storaged.
-  bool inStoraged_ = false;
   // Current service name, used in StatsManager
   std::string serviceName_ = "";
   // Whether to skip the config manager
   bool skipConfig_ = false;
-  // Host role(graph/meta/storage) using this client
+  // Host role(graph/meta/storage) using this client, and UNKNOWN role will not send heartbeat, used
+  // for tools such as upgrader
   cpp2::HostRole role_ = cpp2::HostRole::UNKNOWN;
   // gitInfoSHA of Host using this client
   std::string gitInfoSHA_{""};

diff --git a/src/graph/executor/query/AppendVerticesExecutor.cpp b/src/graph/executor/query/AppendVerticesExecutor.cpp
@@ -70,6 +70,12 @@ Status AppendVerticesExecutor::handleResp(
   auto *av = asNode<AppendVertices>(node());
   auto *vFilter = av->vFilter();
   QueryExpressionContext ctx(qctx()->ectx());
+
+  auto inputIter = qctx()->ectx()->getResult(av->inputVar()).iter();
+  DataSet ds;
+  ds.colNames = av->colNames();
+  ds.rows.reserve(inputIter->size());
+
   for (auto &resp : rpcResp.responses()) {
     if (resp.props_ref().has_value()) {
       auto iter = PropIter(std::make_shared<Value>(std::move(*resp.props_ref())));
@@ -80,26 +86,28 @@ Status AppendVerticesExecutor::handleResp(
             continue;
           }
         }
-        map.emplace(iter.getColumn(kVid), iter.getVertex());
+        if (!av->trackPrevPath()) {  // eg. MATCH (v:Person) RETURN v
+          Row row;
+          row.values.emplace_back(iter.getVertex());
+          ds.rows.emplace_back(std::move(row));
+        } else {
+          map.emplace(iter.getColumn(kVid), iter.getVertex());
+        }
       }
     }
   }
 
-  auto iter = qctx()->ectx()->getResult(av->inputVar()).iter();
-  auto *src = av->src();
+  if (!av->trackPrevPath()) {
+    return finish(ResultBuilder().value(Value(std::move(ds))).state(state).build());
+  }
 
-  DataSet ds;
-  ds.colNames = av->colNames();
-  ds.rows.reserve(iter->size());
-  for (; iter->valid(); iter->next()) {
-    auto dstFound = map.find(src->eval(ctx(iter.get())));
+  auto *src = av->src();
+  for (; inputIter->valid(); inputIter->next()) {
+    auto dstFound = map.find(src->eval(ctx(inputIter.get())));
     if (dstFound == map.end()) {
       continue;
     }
-    Row row;
-    if (av->trackPrevPath()) {
-      row = *iter->row();
-    }
+    Row row = *inputIter->row();
     row.values.emplace_back(dstFound->second);
     ds.rows.emplace_back(std::move(row));
   }

diff --git a/tests/tck/features/match/Base.feature b/tests/tck/features/match/Base.feature
@@ -704,7 +704,7 @@ Feature: Basic match
     When executing query:
       """
       USE nba;
-      MATCH (n:player) RETURN n LIMIT 1;
+      MATCH (n:player) WHERE id(n) == "Boris Diaw" RETURN n;
       """
     Then the result should be, in any order:
       | n                                                   |