From d56c5503cd83cc92d9b98f9c4aeeb4519432916f Mon Sep 17 00:00:00 2001 From: "darion.yaphet" Date: Tue, 7 Dec 2021 17:48:38 +0800 Subject: [PATCH] fix critical27 comment --- src/graph/executor/admin/SpaceExecutor.cpp | 8 +-- src/meta/ActiveHostsMan.cpp | 5 +- .../processors/zone/DropHostsProcessor.cpp | 68 ++++++++++++++---- src/meta/processors/zone/DropHostsProcessor.h | 5 +- .../processors/zone/RenameZoneProcessor.cpp | 3 + src/meta/test/ProcessorTest.cpp | 10 +++ src/parser/MaintainSentences.cpp | 22 +++--- src/parser/Sentence.h | 2 + src/parser/parser.yy | 18 ++--- src/parser/test/ParserTest.cpp | 70 ++++++++++++++++--- 10 files changed, 157 insertions(+), 54 deletions(-) diff --git a/src/graph/executor/admin/SpaceExecutor.cpp b/src/graph/executor/admin/SpaceExecutor.cpp index 801a97d2a7d..f2a452482fc 100644 --- a/src/graph/executor/admin/SpaceExecutor.cpp +++ b/src/graph/executor/admin/SpaceExecutor.cpp @@ -220,14 +220,12 @@ folly::Future ShowCreateSpaceExecutor::execute() { auto fmt = properties.comment_ref().has_value() ? "CREATE SPACE `%s` (partition_num = %d, replica_factor = %d, " "charset = %s, collate = %s, vid_type = %s, atomic_edge = %s" - ")%s" + ") ON %s" " comment = '%s'" : "CREATE SPACE `%s` (partition_num = %d, replica_factor = %d, " "charset = %s, collate = %s, vid_type = %s, atomic_edge = %s" - ")%s"; - auto zoneNames = !properties.get_zone_names().empty() - ? " ON " + folly::join(",", properties.get_zone_names()) - : ""; + ") ON %s"; + auto zoneNames = folly::join(",", properties.get_zone_names()); if (properties.comment_ref().has_value()) { row.values.emplace_back( folly::stringPrintf(fmt, diff --git a/src/meta/ActiveHostsMan.cpp b/src/meta/ActiveHostsMan.cpp index c870c5127b8..5a9b243de3c 100644 --- a/src/meta/ActiveHostsMan.cpp +++ b/src/meta/ActiveHostsMan.cpp @@ -102,11 +102,10 @@ ErrorOr> ActiveHostsMan::getActiv return retCode; } - std::vector machines; + std::unordered_set machines; while (machineIter->valid()) { auto machine = MetaKeyUtils::parseMachineKey(machineIter->key()); - LOG(INFO) << "Machine " << machine; - machines.emplace_back(std::move(machine)); + machines.emplace(std::move(machine)); machineIter->next(); } diff --git a/src/meta/processors/zone/DropHostsProcessor.cpp b/src/meta/processors/zone/DropHostsProcessor.cpp index 12802889906..69be9c521fd 100644 --- a/src/meta/processors/zone/DropHostsProcessor.cpp +++ b/src/meta/processors/zone/DropHostsProcessor.cpp @@ -9,6 +9,8 @@ namespace nebula { namespace meta { void DropHostsProcessor::process(const cpp2::DropHostsReq& req) { + folly::SharedMutex::ReadHolder zHolder(LockUtils::zoneLock()); + folly::SharedMutex::ReadHolder mHolder(LockUtils::machineLock()); auto hosts = req.get_hosts(); if (std::unique(hosts.begin(), hosts.end()) != hosts.end()) { LOG(ERROR) << "Hosts have duplicated element"; @@ -25,6 +27,7 @@ void DropHostsProcessor::process(const cpp2::DropHostsReq& req) { } std::vector data; + std::vector rewriteData; // Check that partition is not held on the host const auto& spacePrefix = MetaKeyUtils::spacePrefix(); auto spaceIterRet = doPrefix(spacePrefix); @@ -77,21 +80,49 @@ void DropHostsProcessor::process(const cpp2::DropHostsReq& req) { auto iter = nebula::value(iterRet).get(); while (iter->valid()) { + auto zoneKey = iter->key(); auto hs = MetaKeyUtils::parseZoneHosts(iter->val()); + // Delete all hosts in the zone if (std::all_of(hs.begin(), hs.end(), [&hosts](auto& h) { return std::find(hosts.begin(), hosts.end(), h) != hosts.end(); })) { - auto zoneName = MetaKeyUtils::parseZoneName(iter->key()); + auto zoneName = MetaKeyUtils::parseZoneName(zoneKey); LOG(INFO) << "Drop zone " << zoneName; - code = checkRelatedSpace(zoneName); - if (code != nebula::cpp2::ErrorCode::SUCCEEDED) { + auto result = checkRelatedSpaceAndCollect(zoneName); + if (!nebula::ok(result)) { + LOG(ERROR) << "Check related space failed"; + code = nebula::error(result); break; } - data.emplace_back(iter->key()); + + const auto& rewrites = nebula::value(result); + rewriteData.insert(rewriteData.end(), rewrites.begin(), rewrites.end()); + data.emplace_back(zoneKey); + } else { + // Delete some hosts in the zone + for (auto& h : hosts) { + auto it = std::find(hs.begin(), hs.end(), h); + if (it != hs.end()) { + hs.erase(it); + } + } + + auto zoneValue = MetaKeyUtils::zoneVal(hs); + LOG(INFO) << "Zone Value: " << zoneValue; + rewriteData.emplace_back(std::move(zoneKey), std::move(zoneValue)); + } + if (code != nebula::cpp2::ErrorCode::SUCCEEDED) { + break; } iter->next(); } + if (code != nebula::cpp2::ErrorCode::SUCCEEDED) { + handleErrorCode(code); + onFinished(); + return; + } + // Detach the machine from cluster for (auto& host : hosts) { auto machineKey = MetaKeyUtils::machineKey(host.host, host.port); @@ -111,10 +142,21 @@ void DropHostsProcessor::process(const cpp2::DropHostsReq& req) { } resp_.set_code(nebula::cpp2::ErrorCode::SUCCEEDED); - doMultiRemove(std::move(data)); + folly::Baton baton; + kvstore_->asyncMultiRemove(kDefaultSpaceId, + kDefaultPartId, + std::move(data), + [this, &baton](nebula::cpp2::ErrorCode result) { + this->handleErrorCode(result); + baton.post(); + }); + baton.wait(); + doSyncPutAndUpdate(std::move(rewriteData)); } -nebula::cpp2::ErrorCode DropHostsProcessor::checkRelatedSpace(const std::string& zoneName) { +ErrorOr> +DropHostsProcessor::checkRelatedSpaceAndCollect(const std::string& zoneName) { + std::vector data; const auto& prefix = MetaKeyUtils::spacePrefix(); auto ret = doPrefix(prefix); if (!nebula::ok(ret)) { @@ -137,19 +179,15 @@ nebula::cpp2::ErrorCode DropHostsProcessor::checkRelatedSpace(const std::string& } else { zones.erase(it); properties.set_zone_names(zones); - rewriteSpaceProperties(iter->key().data(), properties); + + auto spaceKey = iter->key().data(); + auto spaceVal = MetaKeyUtils::spaceVal(properties); + data.emplace_back(std::move(spaceKey), std::move(spaceVal)); } } iter->next(); } - return nebula::cpp2::ErrorCode::SUCCEEDED; -} - -void DropHostsProcessor::rewriteSpaceProperties(const std::string& spaceKey, - const meta::cpp2::SpaceDesc& properties) { - auto spaceVal = MetaKeyUtils::spaceVal(properties); - std::vector data = {{spaceKey, spaceVal}}; - doSyncPutAndUpdate(std::move(data)); + return data; } } // namespace meta diff --git a/src/meta/processors/zone/DropHostsProcessor.h b/src/meta/processors/zone/DropHostsProcessor.h index 41057636c02..8469eb79a45 100644 --- a/src/meta/processors/zone/DropHostsProcessor.h +++ b/src/meta/processors/zone/DropHostsProcessor.h @@ -22,9 +22,8 @@ class DropHostsProcessor : public BaseProcessor { private: explicit DropHostsProcessor(kvstore::KVStore* kvstore) : BaseProcessor(kvstore) {} - nebula::cpp2::ErrorCode checkRelatedSpace(const std::string& zoneName); - - void rewriteSpaceProperties(const std::string& spaceKey, const meta::cpp2::SpaceDesc& properties); + ErrorOr> checkRelatedSpaceAndCollect( + const std::string& zoneName); }; } // namespace meta diff --git a/src/meta/processors/zone/RenameZoneProcessor.cpp b/src/meta/processors/zone/RenameZoneProcessor.cpp index 4dcbf8fe5b5..d3bb2e750b9 100644 --- a/src/meta/processors/zone/RenameZoneProcessor.cpp +++ b/src/meta/processors/zone/RenameZoneProcessor.cpp @@ -9,6 +9,9 @@ namespace nebula { namespace meta { void RenameZoneProcessor::process(const cpp2::RenameZoneReq& req) { + folly::SharedMutex::ReadHolder zHolder(LockUtils::zoneLock()); + folly::SharedMutex::ReadHolder mHolder(LockUtils::machineLock()); + auto originalZoneName = req.get_original_zone_name(); auto originalZoneKey = MetaKeyUtils::zoneKey(originalZoneName); auto originalZoneValueRet = doGet(std::move(originalZoneKey)); diff --git a/src/meta/test/ProcessorTest.cpp b/src/meta/test/ProcessorTest.cpp index 81cffadd621..417e279f9ea 100644 --- a/src/meta/test/ProcessorTest.cpp +++ b/src/meta/test/ProcessorTest.cpp @@ -3303,11 +3303,17 @@ TEST(ProcessorTest, DropHostsTest) { ASSERT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, resp.get_code()); ASSERT_EQ(6, resp.get_zones().size()); ASSERT_EQ("default_zone_127.0.0.1_8987", resp.get_zones()[0].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[0].get_nodes().size()); ASSERT_EQ("default_zone_127.0.0.1_8988", resp.get_zones()[1].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[1].get_nodes().size()); ASSERT_EQ("default_zone_127.0.0.1_8989", resp.get_zones()[2].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[2].get_nodes().size()); ASSERT_EQ("zone_0", resp.get_zones()[3].get_zone_name()); + ASSERT_EQ(2, resp.get_zones()[3].get_nodes().size()); ASSERT_EQ("zone_1", resp.get_zones()[4].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[4].get_nodes().size()); ASSERT_EQ("zone_2", resp.get_zones()[5].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[5].get_nodes().size()); } { // Create Space on cluster, the replica number same with the zone size @@ -3466,9 +3472,13 @@ TEST(ProcessorTest, DropHostsTest) { ASSERT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, resp.get_code()); ASSERT_EQ(4, resp.get_zones().size()); ASSERT_EQ("default_zone_127.0.0.1_8988", resp.get_zones()[0].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[0].get_nodes().size()); ASSERT_EQ("default_zone_127.0.0.1_8989", resp.get_zones()[1].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[1].get_nodes().size()); ASSERT_EQ("zone_0", resp.get_zones()[2].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[2].get_nodes().size()); ASSERT_EQ("zone_2", resp.get_zones()[3].get_zone_name()); + ASSERT_EQ(1, resp.get_zones()[3].get_nodes().size()); } } diff --git a/src/parser/MaintainSentences.cpp b/src/parser/MaintainSentences.cpp index 78cf358e57a..b9ced49661c 100644 --- a/src/parser/MaintainSentences.cpp +++ b/src/parser/MaintainSentences.cpp @@ -379,37 +379,40 @@ std::string MergeZoneSentence::toString() const { buf.reserve(128); buf += "MERGE ZONE "; buf += zoneNames_->toString(); - buf += " INTO "; + buf += " INTO \""; buf += *zoneName_; + buf += "\""; return buf; } std::string DropZoneSentence::toString() const { - return folly::stringPrintf("DROP ZONE %s", zoneName_.get()->c_str()); + return folly::stringPrintf("DROP ZONE \"%s\"", zoneName_.get()->c_str()); } std::string SplitZoneSentence::toString() const { std::string buf; buf.reserve(128); - buf += "SPLIT ZONE "; + buf += "SPLIT ZONE \""; buf += *zoneName_; - buf += " INTO "; + buf += "\" INTO \""; buf += zoneNames_->toString(); + buf += "\""; return buf; } std::string RenameZoneSentence::toString() const { std::string buf; buf.reserve(128); - buf += "RENAME ZONE "; + buf += "RENAME ZONE \""; buf += *originalZoneName_; - buf += " TO "; + buf += "\" TO \""; buf += *zoneName_; + buf += "\""; return buf; } std::string DescribeZoneSentence::toString() const { - return folly::stringPrintf("DESCRIBE ZONE %s", zoneName_.get()->c_str()); + return folly::stringPrintf("DESCRIBE ZONE \"%s\"", zoneName_.get()->c_str()); } std::string ListZonesSentence::toString() const { return folly::stringPrintf("SHOW ZONES"); } @@ -420,11 +423,12 @@ std::string AddHostsIntoZoneSentence::toString() const { buf += "ADD HOSTS "; buf += address_->toString(); if (isNew_) { - buf += " INTO NEW ZONE "; + buf += " INTO NEW ZONE \""; } else { - buf += " INTO ZONE "; + buf += " INTO ZONE \""; } buf += *zoneName_; + buf += "\""; return buf; } diff --git a/src/parser/Sentence.h b/src/parser/Sentence.h index 9af5ebe410d..4e27150d368 100644 --- a/src/parser/Sentence.h +++ b/src/parser/Sentence.h @@ -205,7 +205,9 @@ class ZoneNameList final { std::string toString() const { std::string buf; for (const auto &zone : zones_) { + buf += "\""; buf += *zone; + buf += "\""; buf += ","; } if (!zones_.empty()) { diff --git a/src/parser/parser.yy b/src/parser/parser.yy index f3855af89f0..92bbd5599bc 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -2685,10 +2685,10 @@ add_hosts_sentence : KW_ADD KW_HOSTS host_list { $$ = new AddHostsSentence($3); } - | KW_ADD KW_HOSTS host_list KW_INTO KW_ZONE name_label { + | KW_ADD KW_HOSTS host_list KW_INTO KW_ZONE STRING { $$ = new AddHostsIntoZoneSentence($3, $6, false); } - | KW_ADD KW_HOSTS host_list KW_INTO KW_NEW KW_ZONE name_label { + | KW_ADD KW_HOSTS host_list KW_INTO KW_NEW KW_ZONE STRING { $$ = new AddHostsIntoZoneSentence($3, $7, true); } ; @@ -2701,13 +2701,13 @@ drop_hosts_sentence merge_zone_sentence - : KW_MERGE KW_ZONE zone_name_list KW_INTO name_label { + : KW_MERGE KW_ZONE zone_name_list KW_INTO STRING { $$ = new MergeZoneSentence($3, $5); } ; drop_zone_sentence - : KW_DROP KW_ZONE name_label { + : KW_DROP KW_ZONE STRING { $$ = new DropZoneSentence($3); } ; @@ -2719,16 +2719,16 @@ drop_zone_sentence // ; rename_zone_sentence - : KW_RENAME KW_ZONE name_label KW_TO name_label { + : KW_RENAME KW_ZONE STRING KW_TO STRING { $$ = new RenameZoneSentence($3, $5); } ; desc_zone_sentence - : KW_DESCRIBE KW_ZONE name_label { + : KW_DESCRIBE KW_ZONE STRING { $$ = new DescribeZoneSentence($3); } - | KW_DESC KW_ZONE name_label { + | KW_DESC KW_ZONE STRING { $$ = new DescribeZoneSentence($3); } ; @@ -3293,11 +3293,11 @@ show_config_item ; zone_name_list - : name_label { + : STRING { $$ = new ZoneNameList(); $$->addZone($1); } - | zone_name_list COMMA name_label { + | zone_name_list COMMA STRING { $$ = $1; $$->addZone($3); } diff --git a/src/parser/test/ParserTest.cpp b/src/parser/test/ParserTest.cpp index 1fa8d794979..022ab938d56 100644 --- a/src/parser/test/ParserTest.cpp +++ b/src/parser/test/ParserTest.cpp @@ -227,12 +227,24 @@ TEST_F(ParserTest, SpaceOperation) { { std::string query = "CREATE SPACE default_space(partition_num=9, replica_factor=3) " - "ON group_0"; + "ON \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "CREATE SPACE default_space ON group_0"; + std::string query = + "CREATE SPACE default_space(partition_num=9, replica_factor=3) " + "ON \"zone_0\",\"zone_1\",\"zone_2\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = "CREATE SPACE default_space ON \"zone_0\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = "CREATE SPACE default_space ON \"zone_0\",\"zone_1\",\"zone_2\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } @@ -2768,42 +2780,80 @@ TEST_F(ParserTest, Zone) { ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "ADD HOSTS 127.0.0.1:8989 INTO ZONE zone_0"; + std::string query = "ADD HOSTS 127.0.0.1:8989 INTO ZONE \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "ADD HOSTS 127.0.0.1:8988,127.0.0.1:8989 INTO ZONE zone_0"; + std::string query = "ADD HOSTS 127.0.0.1:8989 INTO ZONE \"default_zone_127.0.0.1_8988\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "DESC ZONE zone_0"; + std::string query = "ADD HOSTS 127.0.0.1:8988,127.0.0.1:8989 INTO ZONE \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "DESCRIBE ZONE zone_0"; + std::string query = + "ADD HOSTS 127.0.0.1:8988,127.0.0.1:8989 INTO ZONE" + " \"default_zone_127.0.0.1_8988\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = "DESC ZONE \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "DROP ZONE zone_0"; + std::string query = "DESCRIBE ZONE \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "MERGE ZONE zone_1,zone_2 INTO zone"; + std::string query = "DROP ZONE \"zone_0\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "MERGE ZONE zone_1,zone_2 INTO zone_1"; + std::string query = "MERGE ZONE \"zone_1\",\"zone_2\" INTO \"zone\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = "MERGE ZONE \"zone_1\",\"zone_2\" INTO \"zone_1\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = + "MERGE ZONE \"default_zone_127.0.0.1_8988\",\"default_zone_127.0.0.1_8989\"" + "INTO \"zone_1\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string query = + "MERGE ZONE \"default_zone_127.0.0.1_8988\",\"default_zone_127.0.0.1_8989\"" + "INTO \"default_zone_127.0.0.1_8989\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + + { + std::string query = "RENAME ZONE \"default_zone_127.0.0.1_8989\" TO \"new_name\""; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + } + + { + std::string query = "RENAME ZONE \"old_name\" TO \"new_name\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } { - std::string query = "RENAME ZONE old_name TO new_name"; + std::string query = "RENAME ZONE \"default_zone_127.0.0.1_8989\" TO \"new_name\""; auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); }