From 9434b395780c4914b0d203160a519869dc1a0006 Mon Sep 17 00:00:00 2001
From: "darion.yaphet" <darion.yaphet@gmail.com>
Date: Tue, 11 Jan 2022 18:45:05 +0800
Subject: [PATCH] fix issue 3675

---
 src/interface/common.thrift                   |  1 +
 .../processors/parts/CreateSpaceProcessor.cpp | 19 ++++++++-----------
 .../processors/parts/CreateSpaceProcessor.h   |  5 +++--
 src/meta/test/ProcessorTest.cpp               |  4 ++--
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/src/interface/common.thrift b/src/interface/common.thrift
index d21f704027a..b8dad9c5aad 100644
--- a/src/interface/common.thrift
+++ b/src/interface/common.thrift
@@ -358,6 +358,7 @@ enum ErrorCode {
     E_CONFLICT                        = -2008,
     E_INVALID_PARM                    = -2009,
     E_WRONGCLUSTER                    = -2010,
+    E_ZONE_NOT_ENOUGH                 = -2011,
 
     E_STORE_FAILURE                   = -2021,
     E_STORE_SEGMENT_ILLEGAL           = -2022,
diff --git a/src/meta/processors/parts/CreateSpaceProcessor.cpp b/src/meta/processors/parts/CreateSpaceProcessor.cpp
index a2b3b0cf396..d5e4e4b1cb0 100644
--- a/src/meta/processors/parts/CreateSpaceProcessor.cpp
+++ b/src/meta/processors/parts/CreateSpaceProcessor.cpp
@@ -146,7 +146,7 @@ void CreateSpaceProcessor::process(const cpp2::CreateSpaceReq& req) {
   int32_t zoneNum = zones.size();
   if (replicaFactor > zoneNum) {
     LOG(ERROR) << "Replication number should less than or equal to zone number.";
-    handleErrorCode(nebula::cpp2::ErrorCode::E_INVALID_PARM);
+    handleErrorCode(nebula::cpp2::ErrorCode::E_ZONE_NOT_ENOUGH);
     onFinished();
     return;
   }
@@ -234,16 +234,16 @@ void CreateSpaceProcessor::process(const cpp2::CreateSpaceReq& req) {
 
     auto pickedZones = std::move(pickedZonesRet).value();
     auto partHostsRet = pickHostsWithZone(pickedZones, zoneHosts);
-    if (!partHostsRet.ok()) {
+    if (!nebula::ok(partHostsRet)) {
       LOG(ERROR) << "Pick hosts with zone failed.";
-      code = nebula::cpp2::ErrorCode::E_INVALID_PARM;
+      code = nebula::error(partHostsRet);
       break;
     }
 
-    auto partHosts = std::move(partHostsRet).value();
+    auto partHosts = nebula::value(partHostsRet);
     if (partHosts.empty()) {
       LOG(ERROR) << "Pick hosts is empty.";
-      code = nebula::cpp2::ErrorCode::E_INVALID_PARM;
+      code = nebula::cpp2::ErrorCode::E_NO_HOSTS;
       break;
     }
 
@@ -268,16 +268,12 @@ void CreateSpaceProcessor::process(const cpp2::CreateSpaceReq& req) {
   LOG(INFO) << "Create space " << spaceName;
 }
 
-StatusOr<Hosts> CreateSpaceProcessor::pickHostsWithZone(
+ErrorOr<nebula::cpp2::ErrorCode, Hosts> CreateSpaceProcessor::pickHostsWithZone(
     const std::vector<std::string>& zones,
     const std::unordered_map<std::string, Hosts>& zoneHosts) {
   Hosts pickedHosts;
   nebula::cpp2::ErrorCode code = nebula::cpp2::ErrorCode::SUCCEEDED;
   for (auto iter = zoneHosts.begin(); iter != zoneHosts.end(); iter++) {
-    if (code != nebula::cpp2::ErrorCode::SUCCEEDED) {
-      break;
-    }
-
     if (iter->second.empty()) {
       LOG(ERROR) << "Zone " << iter->first << " is empty";
       code = nebula::cpp2::ErrorCode::E_INVALID_PARM;
@@ -305,12 +301,13 @@ StatusOr<Hosts> CreateSpaceProcessor::pickHostsWithZone(
       }
     }
 
+    CHECK_CODE_AND_BREAK();
     hostLoading_[picked] += 1;
     pickedHosts.emplace_back(toThriftHost(std::move(picked)));
   }
 
   if (code != nebula::cpp2::ErrorCode::SUCCEEDED) {
-    return Status::Error("Host not found");
+    return code;
   }
   return pickedHosts;
 }
diff --git a/src/meta/processors/parts/CreateSpaceProcessor.h b/src/meta/processors/parts/CreateSpaceProcessor.h
index 67768039dbb..b52ab4ac958 100644
--- a/src/meta/processors/parts/CreateSpaceProcessor.h
+++ b/src/meta/processors/parts/CreateSpaceProcessor.h
@@ -26,8 +26,9 @@ class CreateSpaceProcessor : public BaseProcessor<cpp2::ExecResp> {
       : BaseProcessor<cpp2::ExecResp>(kvstore) {}
 
   // Get the host with the least load in the zone
-  StatusOr<Hosts> pickHostsWithZone(const std::vector<std::string>& zones,
-                                    const std::unordered_map<std::string, Hosts>& zoneHosts);
+  ErrorOr<nebula::cpp2::ErrorCode, Hosts> pickHostsWithZone(
+      const std::vector<std::string>& zones,
+      const std::unordered_map<std::string, Hosts>& zoneHosts);
 
   // Get the zones with the least load
   StatusOr<std::vector<std::string>> pickLightLoadZones(int32_t replicaFactor);
diff --git a/src/meta/test/ProcessorTest.cpp b/src/meta/test/ProcessorTest.cpp
index 7e7f5c3edad..559872ae3b4 100644
--- a/src/meta/test/ProcessorTest.cpp
+++ b/src/meta/test/ProcessorTest.cpp
@@ -3295,7 +3295,7 @@ TEST(ProcessorTest, DropHostsTest) {
     auto f = processor->getFuture();
     processor->process(req);
     auto resp = std::move(f).get();
-    ASSERT_EQ(nebula::cpp2::ErrorCode::E_INVALID_PARM, resp.get_code());
+    ASSERT_EQ(nebula::cpp2::ErrorCode::E_ZONE_NOT_ENOUGH, resp.get_code());
   }
   {
     cpp2::AddHostsIntoZoneReq req;
@@ -3432,7 +3432,7 @@ TEST(ProcessorTest, DropHostsTest) {
     auto f = processor->getFuture();
     processor->process(req);
     auto resp = std::move(f).get();
-    ASSERT_EQ(nebula::cpp2::ErrorCode::E_INVALID_PARM, resp.get_code());
+    ASSERT_EQ(nebula::cpp2::ErrorCode::E_ZONE_NOT_ENOUGH, resp.get_code());
   }
   {
     // Drop hosts which hold partition.