From 5c560b0edebc7e78b7e808dd8a62bfa413776192 Mon Sep 17 00:00:00 2001 From: Hanqing Wu Date: Tue, 26 Dec 2023 16:21:38 +0800 Subject: [PATCH] curvebs(mds): Fix fail to get clone source reference Signed-off-by: Hanqing Wu --- src/mds/server/mds.cpp | 9 +- src/mds/snapshotcloneclient/BUILD | 2 + .../snapshotclone_client.cpp | 105 ++++++++++++------ .../snapshotclone_client.h | 14 ++- .../mock/mock_snapshotclone_client.h | 2 +- .../test_snapshotclone_client.cpp | 48 +++++++- 6 files changed, 133 insertions(+), 47 deletions(-) diff --git a/src/mds/server/mds.cpp b/src/mds/server/mds.cpp index c8fc0fe453..3992503ee3 100644 --- a/src/mds/server/mds.cpp +++ b/src/mds/server/mds.cpp @@ -420,16 +420,17 @@ void MDS::InitNameServerStorage(int mdsCacheCount) { void MDS::InitSnapshotCloneClientOption(SnapshotCloneClientOption *option) { if (!conf_->GetValue("mds.snapshotcloneclient.addr", - &option->snapshotCloneAddr)) { - option->snapshotCloneAddr = ""; - } + &option->snapshotCloneAddr)) { + option->snapshotCloneAddr = ""; + } } void MDS::InitSnapshotCloneClient() { snapshotCloneClient_ = std::make_shared(); SnapshotCloneClientOption snapshotCloneClientOption; InitSnapshotCloneClientOption(&snapshotCloneClientOption); - snapshotCloneClient_->Init(snapshotCloneClientOption); + bool succ = snapshotCloneClient_->Init(snapshotCloneClientOption); + LOG_IF(FATAL, !succ) << "Fail to init snapshotclone client"; } void MDS::InitCurveFS(const CurveFSOption& curveFSOptions) { diff --git a/src/mds/snapshotcloneclient/BUILD b/src/mds/snapshotcloneclient/BUILD index 023da01367..899cf4ab34 100644 --- a/src/mds/snapshotcloneclient/BUILD +++ b/src/mds/snapshotcloneclient/BUILD @@ -34,5 +34,7 @@ cc_library( "//external:json", "//src/common/snapshotclone:curve_snapshotclone", "//proto:nameserver2_cc_proto", + "//src/common:curve_common", + "@com_google_googletest//:gtest_prod", ], ) diff --git a/src/mds/snapshotcloneclient/snapshotclone_client.cpp b/src/mds/snapshotcloneclient/snapshotclone_client.cpp index 3f76d111a7..5a91c00dcf 100644 --- a/src/mds/snapshotcloneclient/snapshotclone_client.cpp +++ b/src/mds/snapshotcloneclient/snapshotclone_client.cpp @@ -23,6 +23,8 @@ #include "src/mds/snapshotcloneclient/snapshotclone_client.h" #include #include +#include +#include "src/common/string_util.h" using curve::snapshotcloneserver::kServiceName; using curve::snapshotcloneserver::kActionStr; @@ -53,39 +55,26 @@ StatusCode SnapshotCloneClient::GetCloneRefStatus(std::string filename, return StatusCode::kSnapshotCloneServerNotInit; } - brpc::Channel channel; - brpc::ChannelOptions option; - option.protocol = "http"; - - std::string url = addr_ - + "/" + kServiceName + "?" - + kActionStr+ "=" + kGetCloneRefStatusAction + "&" - + kVersionStr + "=1&" - + kUserStr + "=" + user + "&" - + kSourceStr + "=" + filename; - - if (channel.Init(url.c_str(), "", &option) != 0) { - LOG(ERROR) << "GetCloneRefStatus, Fail to init channel, url is " << url - << ", filename = " << filename - << ", user = " << user; - return StatusCode::kSnapshotCloneConnectFail; + std::string data; + size_t index = 0; + StatusCode st = StatusCode::KInternalError; + while (index < addrs_.size()) { + st = GetCloneRefStatus(addrs_[index], filename, user, &data); + if (st == StatusCode::kOK) { + break; + } + + LOG(WARNING) << "GetCloneRefStatus, Fail to get status from " + << addrs_[index]; + ++index; } - brpc::Controller cntl; - cntl.http_request().uri() = url.c_str(); - - channel.CallMethod(NULL, &cntl, NULL, NULL, NULL); - if (cntl.Failed()) { - LOG(ERROR) << "GetCloneRefStatus, CallMethod faile, errMsg :" - << cntl.ErrorText() - << ", filename = " << filename - << ", user = " << user; - return StatusCode::KInternalError; + if (index >= addrs_.size()) { + LOG(ERROR) + << "GetCloneRefStatus, Fail to get status from all addresses"; + return st; } - std::stringstream ss; - ss << cntl.response_attachment(); - std::string data = ss.str(); Json::Reader jsonReader; Json::Value jsonObj; if (!jsonReader.parse(data, jsonObj)) { @@ -131,16 +120,66 @@ StatusCode SnapshotCloneClient::GetCloneRefStatus(std::string filename, return StatusCode::kOK; } -void SnapshotCloneClient::Init(const SnapshotCloneClientOption &option) { - if (!option.snapshotCloneAddr.empty()) { - addr_ = option.snapshotCloneAddr; - inited_ = true; +bool SnapshotCloneClient::Init(const SnapshotCloneClientOption &option) { + if (option.snapshotCloneAddr.empty()) { + LOG(WARNING) << "Fail to init snapshot clone client, addr is empty"; + return false; + } + + std::vector addresses; + curve::common::SplitString(option.snapshotCloneAddr, ",", &addresses); + if (addresses.empty()) { + LOG(WARNING) << "Fail to split address"; + return false; } + + addrs_ = std::move(addresses); + inited_ = true; + + return true; } bool SnapshotCloneClient::GetInitStatus() { return inited_; } + +StatusCode SnapshotCloneClient::GetCloneRefStatus(const std::string& addr, + const std::string& filename, + const std::string& user, + std::string* response) { + brpc::Channel channel; + brpc::ChannelOptions option; + option.protocol = "http"; + + std::string url = addr + + "/" + kServiceName + "?" + + kActionStr+ "=" + kGetCloneRefStatusAction + "&" + + kVersionStr + "=1&" + + kUserStr + "=" + user + "&" + + kSourceStr + "=" + filename; + + if (channel.Init(url.c_str(), "", &option) != 0) { + LOG(WARNING) << "GetCloneRefStatus, Fail to init channel, url is " + << url << ", filename = " << filename + << ", user = " << user; + return StatusCode::kSnapshotCloneConnectFail; + } + + brpc::Controller cntl; + cntl.http_request().uri() = url.c_str(); + + channel.CallMethod(NULL, &cntl, NULL, NULL, NULL); + if (cntl.Failed()) { + LOG(WARNING) << "GetCloneRefStatus, CallMethod failed, errMsg :" + << cntl.ErrorText() << ", filename = " << filename + << ", user = " << user; + return StatusCode::KInternalError; + } + + *response = cntl.response_attachment().to_string(); + return StatusCode::kOK; +} + } // namespace snapshotcloneclient } // namespace mds } // namespace curve diff --git a/src/mds/snapshotcloneclient/snapshotclone_client.h b/src/mds/snapshotcloneclient/snapshotclone_client.h index 525f403554..cacd5f391b 100644 --- a/src/mds/snapshotcloneclient/snapshotclone_client.h +++ b/src/mds/snapshotcloneclient/snapshotclone_client.h @@ -23,6 +23,7 @@ #ifndef SRC_MDS_SNAPSHOTCLONECLIENT_SNAPSHOTCLONE_CLIENT_H_ #define SRC_MDS_SNAPSHOTCLONECLIENT_SNAPSHOTCLONE_CLIENT_H_ +#include #include #include #include "src/common/snapshotclone/snapshotclone_define.h" @@ -47,11 +48,11 @@ struct DestFileInfo { class SnapshotCloneClient { public: SnapshotCloneClient() - : addr_(""), inited_(false) {} + : inited_(false) {} virtual ~SnapshotCloneClient() {} - virtual void Init(const SnapshotCloneClientOption &option); + virtual bool Init(const SnapshotCloneClientOption &option); /** * @brief get the clone ref status of a file. As a clone src file, @@ -73,7 +74,14 @@ class SnapshotCloneClient { virtual bool GetInitStatus(); private: - std::string addr_; + FRIEND_TEST(TestSnapshotCloneClient, TestInitWithMultiAddressesSuccess); + + static StatusCode GetCloneRefStatus(const std::string& addr, + const std::string& filename, + const std::string& user, + std::string* response); + + std::vector addrs_; bool inited_; }; diff --git a/test/mds/nameserver2/mock/mock_snapshotclone_client.h b/test/mds/nameserver2/mock/mock_snapshotclone_client.h index 7a61ef700c..994aafcc27 100644 --- a/test/mds/nameserver2/mock/mock_snapshotclone_client.h +++ b/test/mds/nameserver2/mock/mock_snapshotclone_client.h @@ -34,7 +34,7 @@ namespace snapshotcloneclient { class MockSnapshotCloneClient: public SnapshotCloneClient { public: ~MockSnapshotCloneClient() {} - MOCK_METHOD1(Init, void(const SnapshotCloneClientOption &option)); + MOCK_METHOD1(Init, bool(const SnapshotCloneClientOption &option)); MOCK_METHOD4(GetCloneRefStatus, StatusCode(std::string, std::string, CloneRefStatus *, std::vector *)); diff --git a/test/mds/snapshotcloneclient/test_snapshotclone_client.cpp b/test/mds/snapshotcloneclient/test_snapshotclone_client.cpp index d2f62b6d9e..d07bc67206 100644 --- a/test/mds/snapshotcloneclient/test_snapshotclone_client.cpp +++ b/test/mds/snapshotcloneclient/test_snapshotclone_client.cpp @@ -51,6 +51,8 @@ namespace curve { namespace mds { namespace snapshotcloneclient { +const std::string kInvalidAddress = "127.0.0.1:65536"; // NOLINT + class TestSnapshotCloneClient : public ::testing::Test { protected: TestSnapshotCloneClient() {} @@ -89,6 +91,19 @@ TEST_F(TestSnapshotCloneClient, TestInitSuccess) { ASSERT_TRUE(client_->GetInitStatus()); } +TEST_F(TestSnapshotCloneClient, TestInitWithMultiAddressesSuccess) { + uint32_t port = listenAddr_.port; + const std::string addr1 = "127.0.0.1:" + std::to_string(port); + const std::string addr2 = "127.0.0.1:" + std::to_string(port + 1); + option.snapshotCloneAddr = addr1 + "," + addr2; + client_->Init(option); + ASSERT_TRUE(client_->GetInitStatus()); + ASSERT_EQ(2, client_->addrs_.size()); + + std::vector addresses{addr1, addr2}; + ASSERT_EQ(addresses, client_->addrs_); +} + TEST_F(TestSnapshotCloneClient, TestInitFalse) { uint32_t port = listenAddr_.port; option.snapshotCloneAddr = ""; @@ -121,6 +136,21 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseConnectFail) { ASSERT_EQ(ret, StatusCode::kSnapshotCloneConnectFail); } +TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFromAllAddressesFail) { + uint32_t port = listenAddr_.port; + option.snapshotCloneAddr = "127.0.0.1:65536,127.0.0.1:65537"; + client_->Init(option); + ASSERT_TRUE(client_->GetInitStatus()); + + std::string filename = "/file"; + std::string user = "test"; + CloneRefStatus status; + std::vector fileCheckList; + auto ret = client_->GetCloneRefStatus(filename, user, + &status, &fileCheckList); + ASSERT_EQ(ret, StatusCode::kSnapshotCloneConnectFail); +} + TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseCallFail) { uint32_t port = listenAddr_.port; option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(0); @@ -138,7 +168,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseCallFail) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseParseFail) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); @@ -163,7 +194,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseParseFail) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseRetNot0) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); @@ -198,7 +230,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseRetNot0) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseInvalidStatus) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); @@ -234,7 +267,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusFalseInvalidStatus) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusSuccessNoRef) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); @@ -272,7 +306,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusSuccessNoRef) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusSuccessHasRef) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); CloneRefStatus refStatus = CloneRefStatus::kHasRef; @@ -309,7 +344,8 @@ TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusSuccessHasRef) { TEST_F(TestSnapshotCloneClient, TestGetCloneRefStatusSuccessNeedCheck) { uint32_t port = listenAddr_.port; - option.snapshotCloneAddr = "127.0.0.1:" + std::to_string(port); + option.snapshotCloneAddr = + kInvalidAddress + "," + "127.0.0.1:" + std::to_string(port); client_->Init(option); ASSERT_TRUE(client_->GetInitStatus()); CloneRefStatus refStatus = CloneRefStatus::kNeedCheck;