From b7cd414332b2918f5811c1a12721f9dae7b73448 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Fri, 26 Nov 2021 18:09:49 +0800 Subject: [PATCH 1/6] feat - remove version from heartbeat --- src/clients/meta/MetaClient.cpp | 2 +- src/common/utils/MetaKeyUtils.cpp | 22 +++++++ src/common/utils/MetaKeyUtils.h | 6 ++ src/interface/meta.thrift | 3 +- src/meta/processors/admin/HBProcessor.cpp | 3 - .../admin/VerifyClientVersionProcessor.cpp | 8 +++ .../processors/parts/ListHostsProcessor.cpp | 9 ++- src/meta/test/CMakeLists.txt | 15 +++++ src/meta/test/VerifyClientVersionTest.cpp | 66 +++++++++++++++++++ 9 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 src/meta/test/VerifyClientVersionTest.cpp diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index 7576b0f9649..0277fe7dfc0 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -2394,7 +2394,6 @@ folly::Future> MetaClient::heartbeat() { req.set_host(options_.localHost_); req.set_role(options_.role_); req.set_git_info_sha(options_.gitInfoSHA_); - req.set_version(getOriginVersion()); if (options_.role_ == cpp2::HostRole::STORAGE) { if (options_.clusterId_.load() == 0) { options_.clusterId_ = FileBasedClusterIdMan::getClusterIdFromFile(FLAGS_cluster_id_path); @@ -3490,6 +3489,7 @@ bool MetaClient::checkIsPlanKilled(SessionID sessionId, ExecutionPlanID planId) Status MetaClient::verifyVersion() { auto req = cpp2::VerifyClientVersionReq(); + req.set_host(options_.localHost_); folly::Promise> promise; auto future = promise.getFuture(); getResponse( diff --git a/src/common/utils/MetaKeyUtils.cpp b/src/common/utils/MetaKeyUtils.cpp index 3018051273d..89f1b903f6c 100644 --- a/src/common/utils/MetaKeyUtils.cpp +++ b/src/common/utils/MetaKeyUtils.cpp @@ -20,6 +20,7 @@ namespace nebula { static const std::unordered_map> systemTableMaps = { {"users", {"__users__", true}}, {"hosts", {"__hosts__", false}}, + {"versions", {"__versions__", false}}, {"snapshots", {"__snapshots__", false}}, {"configs", {"__configs__", true}}, {"groups", {"__groups__", true}}, @@ -57,6 +58,7 @@ static const std::unordered_map< static const std::string kSpacesTable = tableMaps.at("spaces").first; // NOLINT static const std::string kPartsTable = tableMaps.at("parts").first; // NOLINT static const std::string kHostsTable = systemTableMaps.at("hosts").first; // NOLINT +static const std::string kVersionsTable = systemTableMaps.at("versions").first; // NOLINT static const std::string kTagsTable = tableMaps.at("tags").first; // NOLINT static const std::string kEdgesTable = tableMaps.at("edges").first; // NOLINT static const std::string kIndexesTable = tableMaps.at("indexes").first; // NOLINT @@ -267,6 +269,26 @@ HostAddr MetaKeyUtils::parseHostKeyV2(folly::StringPiece key) { return MetaKeyUtils::deserializeHostAddr(key); } +std::string MetaKeyUtils::versionKey(const HostAddr& h) { + std::string key; + key.append(kVersionsTable.data(), kVersionsTable.size()) + .append(MetaKeyUtils::serializeHostAddr(h)); + return key; +} + +std::string MetaKeyUtils::versionVal(const std::string& version) { + std::string val; + auto versionLen = version.size(); + val.reserve(sizeof(int64_t) + versionLen); + val.append(reinterpret_cast(&version), sizeof(int64_t)).append(version); + return val; +} + +std::string MetaKeyUtils::parseVersion(folly::StringPiece val) { + auto len = *reinterpret_cast(val.data()); + return val.subpiece(sizeof(size_t), len).str(); +} + std::string MetaKeyUtils::leaderKey(std::string addr, Port port) { LOG(ERROR) << "deprecated function\n" << boost::stacktrace::stacktrace(); return leaderKeyV2(addr, port); diff --git a/src/common/utils/MetaKeyUtils.h b/src/common/utils/MetaKeyUtils.h index e01eb2d67fc..270959e06a6 100644 --- a/src/common/utils/MetaKeyUtils.h +++ b/src/common/utils/MetaKeyUtils.h @@ -122,6 +122,12 @@ class MetaKeyUtils final { static HostAddr parseHostKeyV2(folly::StringPiece key); + static std::string versionKey(const HostAddr& h); + + static std::string versionVal(const std::string& version); + + static std::string parseVersion(folly::StringPiece val); + static std::string leaderKey(std::string ip, Port port); static std::string leaderKeyV2(std::string addr, Port port); diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 50afb2462fc..44b920640a8 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -561,8 +561,6 @@ struct HBReq { 4: optional map> (cpp.template = "std::unordered_map") leader_partIds; 5: binary git_info_sha, - // version of binary - 6: optional binary version, } struct IndexFieldDef { @@ -1106,6 +1104,7 @@ struct VerifyClientVersionResp { struct VerifyClientVersionReq { 1: required binary version = common.version; + 3: common.HostAddr host; } service MetaService { diff --git a/src/meta/processors/admin/HBProcessor.cpp b/src/meta/processors/admin/HBProcessor.cpp index 6abdb2eff1b..888b363f304 100644 --- a/src/meta/processors/admin/HBProcessor.cpp +++ b/src/meta/processors/admin/HBProcessor.cpp @@ -46,9 +46,6 @@ void HBProcessor::process(const cpp2::HBReq& req) { } HostInfo info(time::WallClock::fastNowInMilliSec(), req.get_role(), req.get_git_info_sha()); - if (req.version_ref().has_value()) { - info.version_ = *req.version_ref(); - } if (req.leader_partIds_ref().has_value()) { ret = ActiveHostsMan::updateHostInfo(kvstore_, host, info, &*req.leader_partIds_ref()); } else { diff --git a/src/meta/processors/admin/VerifyClientVersionProcessor.cpp b/src/meta/processors/admin/VerifyClientVersionProcessor.cpp index 314ab2e9ec8..71a1da04cc0 100644 --- a/src/meta/processors/admin/VerifyClientVersionProcessor.cpp +++ b/src/meta/processors/admin/VerifyClientVersionProcessor.cpp @@ -5,6 +5,8 @@ #include "meta/processors/admin/VerifyClientVersionProcessor.h" +#include "meta/KVBasedClusterIdMan.h" +#include "meta/MetaVersionMan.h" #include "version/Version.h" DEFINE_bool(enable_client_white_list, true, "Turn on/off the client white list."); @@ -25,6 +27,12 @@ void VerifyClientVersionProcessor::process(const cpp2::VerifyClientVersionReq& r req.get_version().c_str(), FLAGS_client_white_list.c_str())); } else { + auto host = req.get_host(); + auto versionKey = MetaKeyUtils::versionKey(host); + auto versionVal = MetaKeyUtils::versionVal(req.get_version().c_str()); + std::vector versionData; + versionData.emplace_back(std::move(versionKey), std::move(versionVal)); + doSyncPut(versionData); resp_.set_code(nebula::cpp2::ErrorCode::SUCCEEDED); } onFinished(); diff --git a/src/meta/processors/parts/ListHostsProcessor.cpp b/src/meta/processors/parts/ListHostsProcessor.cpp index ef1e115f249..a2c425203bb 100644 --- a/src/meta/processors/parts/ListHostsProcessor.cpp +++ b/src/meta/processors/parts/ListHostsProcessor.cpp @@ -124,9 +124,14 @@ nebula::cpp2::ErrorCode ListHostsProcessor::allHostsWithStatus(cpp2::HostRole ro item.set_role(info.role_); item.set_git_info_sha(info.gitInfoSha_); - if (info.version_.has_value()) { - item.set_version(info.version_.value()); + + auto versionKey = MetaKeyUtils::versionKey(item.get_hostAddr()); + auto versionRet = doGet(versionKey); + if (nebula::ok(versionRet)) { + auto versionVal = MetaKeyUtils::parseVersion(value(versionRet)); + item.set_version(versionVal); } + if (now - info.lastHBTimeInMilliSec_ < FLAGS_removed_threshold_sec * 1000) { // If meta didn't receive heartbeat with 2 periods, regard hosts as // offline. Same as ActiveHostsMan::getActiveHosts diff --git a/src/meta/test/CMakeLists.txt b/src/meta/test/CMakeLists.txt index cf8bc5567f6..e8591bae05a 100644 --- a/src/meta/test/CMakeLists.txt +++ b/src/meta/test/CMakeLists.txt @@ -254,3 +254,18 @@ nebula_add_test( wangle gtest ) + +nebula_add_test( + NAME + verify_client_version_test + SOURCES + VerifyClientVersionTest.cpp + OBJECTS + ${meta_test_deps} + LIBRARIES + ${ROCKSDB_LIBRARIES} + ${THRIFT_LIBRARIES} + ${PROXYGEN_LIBRARIES} + wangle + gtest +) diff --git a/src/meta/test/VerifyClientVersionTest.cpp b/src/meta/test/VerifyClientVersionTest.cpp new file mode 100644 index 00000000000..a073eeb1da9 --- /dev/null +++ b/src/meta/test/VerifyClientVersionTest.cpp @@ -0,0 +1,66 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#include + +#include "common/base/Base.h" +#include "common/fs/TempDir.h" +#include "meta/processors/admin/HBProcessor.h" +#include "meta/processors/admin/VerifyClientVersionProcessor.h" +#include "meta/processors/parts/ListHostsProcessor.h" +#include "meta/test/TestUtils.h" + +namespace nebula { +namespace meta { + +TEST(VerifyClientVersionTest, VersionTest) { + fs::TempDir rootPath("/tmp/VersionTest.XXXXXX"); + std::unique_ptr kv(MockCluster::initMetaKV(rootPath.path())); + { + for (auto i = 0; i < 5; i++) { + auto req = cpp2::VerifyClientVersionReq(); + req.set_host(HostAddr(std::to_string(i), i)); + auto* processor = VerifyClientVersionProcessor::instance(kv.get()); + auto f = processor->getFuture(); + processor->process(req); + auto resp = std::move(f).get(); + ASSERT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, resp.get_code()); + } + } + { + const ClusterID kClusterId = 10; + for (auto i = 0; i < 5; i++) { + cpp2::HBReq req; + req.set_role(cpp2::HostRole::GRAPH); + req.set_host(HostAddr(std::to_string(i), i)); + req.set_cluster_id(kClusterId); + auto* processor = HBProcessor::instance(kv.get(), nullptr, kClusterId); + auto f = processor->getFuture(); + processor->process(req); + auto resp = std::move(f).get(); + ASSERT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, resp.get_code()); + } + } + { + auto req = cpp2::ListHostsReq(); + req.set_type(cpp2::ListHostType::GRAPH); + auto* processor = ListHostsProcessor::instance(kv.get()); + auto f = processor->getFuture(); + processor->process(req); + auto resp = std::move(f).get(); + ASSERT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, resp.get_code()); + ASSERT_EQ(resp.get_hosts().size(), 5); + } +} + +} // namespace meta +} // namespace nebula + +int main(int argc, char** argv) { + testing::InitGoogleTest(&argc, argv); + folly::init(&argc, &argv, true); + google::SetStderrLogging(google::INFO); + return RUN_ALL_TESTS(); +} From 061ec98801b33dea1b55745ce9278af05c9a4689 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Mon, 29 Nov 2021 16:40:46 +0800 Subject: [PATCH 2/6] style - remove unused header --- src/meta/processors/admin/VerifyClientVersionProcessor.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/meta/processors/admin/VerifyClientVersionProcessor.cpp b/src/meta/processors/admin/VerifyClientVersionProcessor.cpp index 71a1da04cc0..d7328b5b145 100644 --- a/src/meta/processors/admin/VerifyClientVersionProcessor.cpp +++ b/src/meta/processors/admin/VerifyClientVersionProcessor.cpp @@ -5,8 +5,6 @@ #include "meta/processors/admin/VerifyClientVersionProcessor.h" -#include "meta/KVBasedClusterIdMan.h" -#include "meta/MetaVersionMan.h" #include "version/Version.h" DEFINE_bool(enable_client_white_list, true, "Turn on/off the client white list."); From 6bfb30f6041e42d81deef82015866660ec063188 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Tue, 30 Nov 2021 18:16:15 +0800 Subject: [PATCH 3/6] add test cases --- src/meta/test/VerifyClientVersionTest.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/meta/test/VerifyClientVersionTest.cpp b/src/meta/test/VerifyClientVersionTest.cpp index a073eeb1da9..0819943a987 100644 --- a/src/meta/test/VerifyClientVersionTest.cpp +++ b/src/meta/test/VerifyClientVersionTest.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2018 vesoft inc. All rights reserved. +/* Copyright (c) 2021 vesoft inc. All rights reserved. * * This source code is licensed under Apache 2.0 License. */ @@ -18,6 +18,17 @@ namespace meta { TEST(VerifyClientVersionTest, VersionTest) { fs::TempDir rootPath("/tmp/VersionTest.XXXXXX"); std::unique_ptr kv(MockCluster::initMetaKV(rootPath.path())); + { + for (auto i = 0; i < 5; i++) { + auto req = cpp2::VerifyClientVersionReq(); + req.set_version("1.0.1"); + auto* processor = VerifyClientVersionProcessor::instance(kv.get()); + auto f = processor->getFuture(); + processor->process(req); + auto resp = std::move(f).get(); + ASSERT_EQ(nebula::cpp2::ErrorCode::E_CLIENT_SERVER_INCOMPATIBLE, resp.get_code()); + } + } { for (auto i = 0; i < 5; i++) { auto req = cpp2::VerifyClientVersionReq(); @@ -32,7 +43,7 @@ TEST(VerifyClientVersionTest, VersionTest) { { const ClusterID kClusterId = 10; for (auto i = 0; i < 5; i++) { - cpp2::HBReq req; + auto req = cpp2::HBReq(); req.set_role(cpp2::HostRole::GRAPH); req.set_host(HostAddr(std::to_string(i), i)); req.set_cluster_id(kClusterId); From 5e5b2a5c5b4e12104dffed9c1003effde7f03564 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Tue, 30 Nov 2021 18:17:19 +0800 Subject: [PATCH 4/6] add test case --- src/meta/test/VerifyClientVersionTest.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/meta/test/VerifyClientVersionTest.cpp b/src/meta/test/VerifyClientVersionTest.cpp index 0819943a987..9f72f559e00 100644 --- a/src/meta/test/VerifyClientVersionTest.cpp +++ b/src/meta/test/VerifyClientVersionTest.cpp @@ -19,15 +19,13 @@ TEST(VerifyClientVersionTest, VersionTest) { fs::TempDir rootPath("/tmp/VersionTest.XXXXXX"); std::unique_ptr kv(MockCluster::initMetaKV(rootPath.path())); { - for (auto i = 0; i < 5; i++) { - auto req = cpp2::VerifyClientVersionReq(); - req.set_version("1.0.1"); - auto* processor = VerifyClientVersionProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(nebula::cpp2::ErrorCode::E_CLIENT_SERVER_INCOMPATIBLE, resp.get_code()); - } + auto req = cpp2::VerifyClientVersionReq(); + req.set_version("1.0.1"); + auto* processor = VerifyClientVersionProcessor::instance(kv.get()); + auto f = processor->getFuture(); + processor->process(req); + auto resp = std::move(f).get(); + ASSERT_EQ(nebula::cpp2::ErrorCode::E_CLIENT_SERVER_INCOMPATIBLE, resp.get_code()); } { for (auto i = 0; i < 5; i++) { From 00b0cd03107744e94d41d48c952de59290eae0df Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Thu, 2 Dec 2021 11:00:30 +0800 Subject: [PATCH 5/6] remove version_ from HostInfo --- src/meta/ActiveHostsMan.h | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/meta/ActiveHostsMan.h b/src/meta/ActiveHostsMan.h index 2412c351d9b..7c1e4e9c26f 100644 --- a/src/meta/ActiveHostsMan.h +++ b/src/meta/ActiveHostsMan.h @@ -33,8 +33,6 @@ struct HostInfo { int64_t lastHBTimeInMilliSec_ = 0; cpp2::HostRole role_{cpp2::HostRole::UNKNOWN}; std::string gitInfoSha_; - // version of binary - folly::Optional version_; static HostInfo decode(const folly::StringPiece& data) { if (data.size() == sizeof(int64_t)) { @@ -71,12 +69,6 @@ struct HostInfo { if (!info.gitInfoSha_.empty()) { encode.append(info.gitInfoSha_.data(), len); } - - if (info.version_.has_value()) { - len = info.version_.value().size(); - encode.append(reinterpret_cast(&len), sizeof(std::size_t)); - encode.append(info.version_.value().data(), len); - } return encode; } @@ -104,20 +96,6 @@ struct HostInfo { } info.gitInfoSha_ = std::string(data.data() + offset, len); - offset += len; - - if (offset == data.size()) { - return info; - } - - len = *reinterpret_cast(data.data() + offset); - offset += sizeof(size_t); - - if (offset + len > data.size()) { - FLOG_FATAL("decode out of range, offset=%zu, actual=%zu", offset, data.size()); - } - - info.version_ = std::string(data.data() + offset, len); return info; } }; From 9ece20f6d55d0f40e320daafce6cf215d9feb2ba Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Thu, 2 Dec 2021 13:40:24 +0800 Subject: [PATCH 6/6] style --- src/interface/meta.thrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 44b920640a8..e107d2064c3 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -1104,7 +1104,7 @@ struct VerifyClientVersionResp { struct VerifyClientVersionReq { 1: required binary version = common.version; - 3: common.HostAddr host; + 2: common.HostAddr host; } service MetaService {