From 51947269c39bc3ab4b488c8ff4b259303a6b513d Mon Sep 17 00:00:00 2001 From: "endy.li" <25311962+heroicNeZha@users.noreply.github.com> Date: Tue, 7 Dec 2021 09:58:51 +0800 Subject: [PATCH] Move version info outside of HB (#3378) * feat - remove version from heartbeat * style - remove unused header * add test cases * add test case * remove version_ from HostInfo * style Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> --- src/clients/meta/MetaClient.cpp | 2 +- src/common/utils/MetaKeyUtils.cpp | 22 ++++++ src/common/utils/MetaKeyUtils.h | 6 ++ src/interface/meta.thrift | 5 +- src/meta/ActiveHostsMan.h | 22 ------ src/meta/processors/admin/HBProcessor.cpp | 3 - .../admin/VerifyClientVersionProcessor.cpp | 6 ++ .../processors/parts/ListHostsProcessor.cpp | 9 ++- src/meta/test/CMakeLists.txt | 15 ++++ src/meta/test/VerifyClientVersionTest.cpp | 75 +++++++++++++++++++ 10 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 src/meta/test/VerifyClientVersionTest.cpp diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index 4ecfe498f40..676b923adbb 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); @@ -3505,6 +3504,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 b070cf40731..051ce19d025 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}}, @@ -58,6 +59,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 @@ -269,6 +271,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 29fa715771b..daf934599ea 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 670d0337668..f9bd4e1f92d 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -565,9 +565,7 @@ struct HBReq { 4: optional map> (cpp.template = "std::unordered_map") leader_partIds; 5: binary git_info_sha, - // version of binary - 6: optional binary version, - 7: optional map + 6: optional map (cpp.template = "std::unordered_map")> (cpp.template = "std::unordered_map") disk_parts; } @@ -1113,6 +1111,7 @@ struct VerifyClientVersionResp { struct VerifyClientVersionReq { 1: required binary version = common.version; + 2: common.HostAddr host; } service MetaService { 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; } }; diff --git a/src/meta/processors/admin/HBProcessor.cpp b/src/meta/processors/admin/HBProcessor.cpp index beebe9b4810..04e806cb320 100644 --- a/src/meta/processors/admin/HBProcessor.cpp +++ b/src/meta/processors/admin/HBProcessor.cpp @@ -67,9 +67,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..d7328b5b145 100644 --- a/src/meta/processors/admin/VerifyClientVersionProcessor.cpp +++ b/src/meta/processors/admin/VerifyClientVersionProcessor.cpp @@ -25,6 +25,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..9f72f559e00 --- /dev/null +++ b/src/meta/test/VerifyClientVersionTest.cpp @@ -0,0 +1,75 @@ +/* Copyright (c) 2021 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())); + { + 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(); + 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++) { + auto req = cpp2::HBReq(); + 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(); +}