curvebs(mds): Fix fail to get clone source reference

Signed-off-by: Hanqing Wu <[email protected]>

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<SnapshotCloneClient>();
     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) {

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",
     ],
 )

src/mds/snapshotcloneclient/snapshotclone_client.cpp

@@ -23,6 +23,8 @@
 #include "src/mds/snapshotcloneclient/snapshotclone_client.h"
 #include <brpc/channel.h>
 #include <json/json.h>
+#include <utility>
+#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<std::string> 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

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 <gtest/gtest_prod.h>
 #include <string>
 #include <vector>
 #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<std::string> addrs_;
     bool inited_;
 };
 

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<DestFileInfo> *));

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<std::string> 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<DestFileInfo> 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;