From 96db138de1487c64adef3058fdf16ab2b157df55 Mon Sep 17 00:00:00 2001 From: Sophie <84560950+Sophie-Xie@users.noreply.github.com> Date: Wed, 9 Feb 2022 10:28:47 +0800 Subject: [PATCH] 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 <13706157+critical27@users.noreply.github.com> Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com> Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com> --- .github/workflows/rc.yml | 2 +- src/clients/meta/MetaClient.cpp | 22 +++++++------ src/clients/meta/MetaClient.h | 6 ++-- .../executor/query/AppendVerticesExecutor.cpp | 32 ++++++++++++------- tests/tck/features/match/Base.feature | 2 +- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/.github/workflows/rc.yml b/.github/workflows/rc.yml index f4d4ee37132..51c1d3bc73b 100644 --- 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 index 7c50f8ab120..27b9f68c28e 100644 --- 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 index 3504898aba1..f747ad723f9 100644 --- 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_{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 index 43c42e49325..a092182a8c8 100644 --- a/src/graph/executor/query/AppendVerticesExecutor.cpp +++ b/src/graph/executor/query/AppendVerticesExecutor.cpp @@ -70,6 +70,12 @@ Status AppendVerticesExecutor::handleResp( auto *av = asNode(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(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 index c5b99b14ad5..6964d1e0696 100644 --- 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 |