From c358d53090ee96c51c607eb9f9736207abd85a14 Mon Sep 17 00:00:00 2001 From: Doodle <13706157+critical27@users.noreply.github.com> Date: Wed, 28 Aug 2019 07:34:17 +0800 Subject: [PATCH] Fix graph failed to register config to meta. (#792) * register gflags in meta client * parse json * try register at first operation * address @laura-ding's comments * fix ut, address comments * address laura-dings's comments * address comments * address @dangleptr's comments, update gflags.json * address @dangleptr's comments --- package/nebula.spec | 3 + share/CMakeLists.txt | 11 +++ share/resources/completion.json | 12 ++- share/resources/gflags.json | 46 ++++++++++ src/graph/CMakeLists.txt | 1 + src/graph/ExecutionEngine.cpp | 1 - src/graph/test/ConfigTest.cpp | 41 +++++---- src/interface/meta.thrift | 1 + src/kvstore/plugins/hbase/test/CMakeLists.txt | 2 + src/kvstore/test/CMakeLists.txt | 1 + src/meta/CMakeLists.txt | 4 + src/meta/ClientBasedGflagsManager.cpp | 22 +---- src/meta/ClientBasedGflagsManager.h | 14 +-- src/meta/GflagsManager.cpp | 88 ++++++++++++++++--- src/meta/GflagsManager.h | 18 ++-- src/meta/KVBasedGflagsManager.cpp | 39 ++------ src/meta/KVBasedGflagsManager.h | 12 +-- src/meta/client/MetaClient.cpp | 51 +++++------ src/meta/client/MetaClient.h | 12 ++- src/meta/test/CMakeLists.txt | 14 ++- src/meta/test/ConfigManTest.cpp | 27 +++--- src/storage/StorageServer.cpp | 1 - src/storage/test/CMakeLists.txt | 2 + src/tools/storage-perf/CMakeLists.txt | 1 + 24 files changed, 263 insertions(+), 161 deletions(-) create mode 100644 share/resources/gflags.json diff --git a/package/nebula.spec b/package/nebula.spec index a45ea1ce689..3867ae3d7d9 100755 --- a/package/nebula.spec +++ b/package/nebula.spec @@ -81,6 +81,7 @@ Group: Applications/Databases %attr(0755,root,root) %{_bindir}/nebula-metad %attr(0644,root,root) %{_sysconfdir}/nebula-metad.conf.default %attr(0755,root,root) %{_datadir}/nebula-metad.service +%attr(0644,root,root) %{_resourcesdir}/gflags.json # After install, if config file is non-existent, copy default config file %post metad @@ -94,6 +95,7 @@ fi %attr(0755,root,root) %{_bindir}/nebula-graphd %attr(0644,root,root) %config%{_sysconfdir}/nebula-graphd.conf.default %attr(0755,root,root) %{_datadir}/nebula-graphd.service +%attr(0644,root,root) %{_resourcesdir}/gflags.json %post graphd if [[ ! -f %{_install_dir}/etc/nebula-graphd.conf ]]; then @@ -106,6 +108,7 @@ fi %attr(0755,root,root) %{_bindir}/nebula-storaged %attr(0644,root,root) %config%{_sysconfdir}/nebula-storaged.conf.default %attr(0755,root,root) %{_datadir}/nebula-storaged.service +%attr(0644,root,root) %{_resourcesdir}/gflags.json %post storaged if [[ ! -f %{_install_dir}/etc/nebula-storaged.conf ]]; then diff --git a/share/CMakeLists.txt b/share/CMakeLists.txt index e8985ff1405..43ee9828268 100644 --- a/share/CMakeLists.txt +++ b/share/CMakeLists.txt @@ -8,3 +8,14 @@ install( DESTINATION share/resources ) + +install( + FILES + resources/gflags.json + PERMISSIONS + OWNER_WRITE OWNER_READ + GROUP_READ + WORLD_READ + DESTINATION + share/resources +) diff --git a/share/resources/completion.json b/share/resources/completion.json index 3d1d54e4e5e..7ec984e3283 100644 --- a/share/resources/completion.json +++ b/share/resources/completion.json @@ -12,7 +12,7 @@ }, "SHOW" : { "sub_keywords": [ - "HOSTS", "TAGS", "EDGES", "SPACES", "USERS", "ROLES", "USER" + "HOSTS", "TAGS", "EDGES", "SPACES", "USERS", "ROLES", "USER", "VARIABLES" ] }, "DESCRIBE" : { @@ -76,6 +76,16 @@ "INTERSECT": { }, "UNION": { + }, + "UPDATE": { + "sub_keywords": [ + "VARIABLES" + ] + }, + "GET": { + "sub_keywords": [ + "VARIABLES" + ] } }, "typenames": [ diff --git a/share/resources/gflags.json b/share/resources/gflags.json new file mode 100644 index 00000000000..abf01a82a60 --- /dev/null +++ b/share/resources/gflags.json @@ -0,0 +1,46 @@ +{ + "IMMUTABLE": [ + ], + "REBOOT": [ + ], + "MUTABLE": [ + "load_data_interval_secs" + ], + "IGNORED": [ + "logging", + "flagfile", + "fromenv", + "tryfromenv", + "undefok", + "tab_completion_columns", + "tab_completion_word", + "alsologtostderr", + "drop_log_memory", + "log_backtrace_at", + "log_dir", + "log_link", + "log_prefix", + "logbuflevel", + "logbufsecs", + "logemaillevel", + "logfile_mode", + "logmailer", + "logtostderr", + "max_log_size", + "minloglevel", + "stderrthreshold", + "stop_logging_if_full_disk", + "symbolize_stacktrace", + "vmodule", + "v", + "help", + "helpon", + "helpxml", + "helpfull", + "helpmatch", + "helpshort", + "alsologtoemail", + "version", + "gflags_mode_json" + ] +} diff --git a/src/graph/CMakeLists.txt b/src/graph/CMakeLists.txt index 7df169c38e5..7d20bb11161 100644 --- a/src/graph/CMakeLists.txt +++ b/src/graph/CMakeLists.txt @@ -51,6 +51,7 @@ add_dependencies( graph_thrift_obj meta_thrift_obj meta_client + gflags_man_obj schema_obj stats_obj process_obj diff --git a/src/graph/ExecutionEngine.cpp b/src/graph/ExecutionEngine.cpp index c44f6e2f3f6..9af8ca8ce5e 100644 --- a/src/graph/ExecutionEngine.cpp +++ b/src/graph/ExecutionEngine.cpp @@ -40,7 +40,6 @@ Status ExecutionEngine::init(std::shared_ptr ioExec schemaManager_->init(metaClient_.get()); gflagsManager_ = std::make_unique(metaClient_.get()); - gflagsManager_->init(); storage_ = std::make_unique(ioExecutor, metaClient_.get()); return Status::OK(); diff --git a/src/graph/test/ConfigTest.cpp b/src/graph/test/ConfigTest.cpp index 9be63c525fb..833cf836fca 100644 --- a/src/graph/test/ConfigTest.cpp +++ b/src/graph/test/ConfigTest.cpp @@ -29,55 +29,64 @@ class ConfigTest : public TestBase { } }; -void mockRegisterGflags(meta::ClientBasedGflagsManager* cfgMan) { +std::vector mockRegisterGflags() { + std::vector configItems; { auto module = meta::cpp2::ConfigModule::STORAGE; auto type = meta::cpp2::ConfigType::INT64; int64_t value = 0L; - cfgMan->registerConfig(module, "k0", type, meta::cpp2::ConfigMode::IMMUTABLE, - meta::toThriftValueStr(type, value)).get(); - cfgMan->registerConfig(module, "k1", type, meta::cpp2::ConfigMode::MUTABLE, - meta::toThriftValueStr(type, value)).get(); + configItems.emplace_back(meta::toThriftConfigItem(module, "k0", type, + meta::cpp2::ConfigMode::IMMUTABLE, + meta::toThriftValueStr(type, value))); + configItems.emplace_back(meta::toThriftConfigItem(module, "k1", type, + meta::cpp2::ConfigMode::MUTABLE, + meta::toThriftValueStr(type, value))); } { auto module = meta::cpp2::ConfigModule::STORAGE; auto type = meta::cpp2::ConfigType::BOOL; auto mode = meta::cpp2::ConfigMode::MUTABLE; bool value = false; - cfgMan->registerConfig(module, "k2", type, mode, meta::toThriftValueStr(type, value)).get(); + configItems.emplace_back(meta::toThriftConfigItem(module, "k2", type, mode, + meta::toThriftValueStr(type, value))); } { auto module = meta::cpp2::ConfigModule::META; auto mode = meta::cpp2::ConfigMode::MUTABLE; auto type = meta::cpp2::ConfigType::DOUBLE; double value = 1.0; - cfgMan->registerConfig(module, "k1", type, mode, meta::toThriftValueStr(type, value)).get(); + configItems.emplace_back(meta::toThriftConfigItem(module, "k1", type, mode, + meta::toThriftValueStr(type, value))); } { auto module = meta::cpp2::ConfigModule::META; auto mode = meta::cpp2::ConfigMode::MUTABLE; auto type = meta::cpp2::ConfigType::STRING; std::string value = "nebula"; - cfgMan->registerConfig(module, "k2", type, mode, meta::toThriftValueStr(type, value)).get(); + configItems.emplace_back(meta::toThriftConfigItem(module, "k2", type, mode, + meta::toThriftValueStr(type, value))); } { auto type = meta::cpp2::ConfigType::STRING; auto mode = meta::cpp2::ConfigMode::MUTABLE; std::string value = "nebula"; - cfgMan->registerConfig(meta::cpp2::ConfigModule::GRAPH, "k3", - type, mode, meta::toThriftValueStr(type, value)).get(); - cfgMan->registerConfig(meta::cpp2::ConfigModule::META, "k3", - type, mode, meta::toThriftValueStr(type, value)).get(); - cfgMan->registerConfig(meta::cpp2::ConfigModule::STORAGE, "k3", - type, mode, meta::toThriftValueStr(type, value)).get(); - } + configItems.emplace_back(meta::toThriftConfigItem(meta::cpp2::ConfigModule::GRAPH, "k3", + type, mode, meta::toThriftValueStr(type, value))); + configItems.emplace_back(meta::toThriftConfigItem(meta::cpp2::ConfigModule::META, "k3", + type, mode, meta::toThriftValueStr(type, value))); + configItems.emplace_back(meta::toThriftConfigItem(meta::cpp2::ConfigModule::STORAGE, "k3", + type, mode, meta::toThriftValueStr(type, value))); + } + return configItems; } TEST_F(ConfigTest, ConfigTest) { auto client = gEnv->getClient(); ASSERT_NE(nullptr, client); - mockRegisterGflags(gEnv->gflagsManager()); + auto configItems = mockRegisterGflags(); + auto ret = gEnv->gflagsManager()->registerGflags(std::move(configItems)); + ASSERT_TRUE(ret.ok()); // set/get without declaration { diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 56d691b89b1..e4de7a3372d 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -456,6 +456,7 @@ enum ConfigMode { IMMUTABLE = 0x00, REBOOT = 0x01, MUTABLE = 0x02, + IGNORED = 0x03, } (cpp.enum_strict) struct ConfigItem { diff --git a/src/kvstore/plugins/hbase/test/CMakeLists.txt b/src/kvstore/plugins/hbase/test/CMakeLists.txt index 7afb3e51bd9..4fc70ca6a0d 100644 --- a/src/kvstore/plugins/hbase/test/CMakeLists.txt +++ b/src/kvstore/plugins/hbase/test/CMakeLists.txt @@ -17,6 +17,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -44,6 +45,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} diff --git a/src/kvstore/test/CMakeLists.txt b/src/kvstore/test/CMakeLists.txt index d395d7a9ad6..548d4bc3654 100644 --- a/src/kvstore/test/CMakeLists.txt +++ b/src/kvstore/test/CMakeLists.txt @@ -13,6 +13,7 @@ set(KVSTORE_TEST_LIBS $ $ $ + $ ) nebula_add_test( diff --git a/src/meta/CMakeLists.txt b/src/meta/CMakeLists.txt index 73661900333..6c7a2ea7087 100644 --- a/src/meta/CMakeLists.txt +++ b/src/meta/CMakeLists.txt @@ -58,6 +58,10 @@ nebula_add_library( meta_client OBJECT client/MetaClient.cpp ) +add_dependencies( + meta_client + gflags_man_obj +) add_library( gflags_man_obj OBJECT diff --git a/src/meta/ClientBasedGflagsManager.cpp b/src/meta/ClientBasedGflagsManager.cpp index f23935e4ee3..660f8bc250c 100644 --- a/src/meta/ClientBasedGflagsManager.cpp +++ b/src/meta/ClientBasedGflagsManager.cpp @@ -18,12 +18,6 @@ ClientBasedGflagsManager::~ClientBasedGflagsManager() { metaClient_ = nullptr; } -Status ClientBasedGflagsManager::init() { - module_ = metaClient_->getGflagsModule(); - declareGflags(); - return registerGflags(); -} - template folly::Future> ClientBasedGflagsManager::set(const cpp2::ConfigModule& module, const std::string& name, @@ -70,20 +64,10 @@ ClientBasedGflagsManager::listConfigs(const cpp2::ConfigModule& module) { return metaClient_->listConfigs(module); } -folly::Future> -ClientBasedGflagsManager::registerConfig(const cpp2::ConfigModule& module, const std::string& name, - const cpp2::ConfigType& type, const cpp2::ConfigMode& mode, - const std::string& value) { - auto item = toThriftConfigItem(module, name, type, mode, value); - std::vector items; - items.emplace_back(std::move(item)); - return metaClient_->regConfig(items); -} - -Status ClientBasedGflagsManager::registerGflags() { - auto status = metaClient_->regConfig(gflagsDeclared_).get(); +Status ClientBasedGflagsManager::registerGflags(const std::vector& items) { + auto status = metaClient_->regConfig(items).get(); if (status.ok()) { - LOG(INFO) << "Register gflags ok " << gflagsDeclared_.size(); + LOG(INFO) << "Register gflags ok " << items.size(); return Status::OK(); } return Status::Error("Register gflags failed"); diff --git a/src/meta/ClientBasedGflagsManager.h b/src/meta/ClientBasedGflagsManager.h index 988dc85c95e..f602fac67f0 100644 --- a/src/meta/ClientBasedGflagsManager.h +++ b/src/meta/ClientBasedGflagsManager.h @@ -8,7 +8,6 @@ #define META_CLIENTBASEDGFLAGSMANAGER_H_ #include "base/Base.h" -#include #include "meta/GflagsManager.h" #include "meta/client/MetaClient.h" @@ -16,9 +15,6 @@ namespace nebula { namespace meta { class ClientBasedGflagsManager : public GflagsManager { - FRIEND_TEST(ConfigManTest, MetaConfigManTest); - FRIEND_TEST(ConfigManTest, MockConfigTest); - public: explicit ClientBasedGflagsManager(MetaClient *metaClient); @@ -35,17 +31,9 @@ class ClientBasedGflagsManager : public GflagsManager { folly::Future>> listConfigs(const cpp2::ConfigModule& module) override; - folly::Future> registerConfig(const cpp2::ConfigModule& module, - const std::string& name, - const cpp2::ConfigType& type, - const cpp2::ConfigMode& mode, - const std::string& defaultValue) override; - - Status init() override; + Status registerGflags(const std::vector& items); private: - Status registerGflags(); - template folly::Future> set(const cpp2::ConfigModule& module, const std::string& name, const cpp2::ConfigType& type, const ValueType& value); diff --git a/src/meta/GflagsManager.cpp b/src/meta/GflagsManager.cpp index 1d392002ae4..1d99901553e 100644 --- a/src/meta/GflagsManager.cpp +++ b/src/meta/GflagsManager.cpp @@ -5,6 +5,10 @@ */ #include "meta/GflagsManager.h" +#include "base/Configuration.h" +#include "fs/FileUtils.h" + +DEFINE_string(gflags_mode_json, "share/resources/gflags.json", "gflags mode json for service"); namespace nebula { namespace meta { @@ -23,21 +27,64 @@ std::string GflagsManager::gflagsValueToThriftValue( return flag.current_value; } -void GflagsManager::declareGflags() { - // declare all gflags in ClientBasedGflagsManager +std::unordered_map +GflagsManager::parseConfigJson(const std::string& path) { + std::unordered_map configModeMap; + Configuration conf; + if (!conf.parseFromFile(path).ok()) { + LOG(ERROR) << "Load gflags json failed"; + return configModeMap; + } + static std::vector keys = {"IMMUTABLE", "REBOOT", "MUTABLE", "IGNORED"}; + static std::vector modes = {cpp2::ConfigMode::IMMUTABLE, + cpp2::ConfigMode::REBOOT, + cpp2::ConfigMode::MUTABLE, + cpp2::ConfigMode::IGNORED}; + for (size_t i = 0; i < keys.size(); i++) { + std::vector values; + if (!conf.fetchAsStringArray(keys[i].c_str(), values).ok()) { + continue; + } + cpp2::ConfigMode mode = modes[i]; + for (const auto& name : values) { + configModeMap[name] = mode; + } + } + return configModeMap; +} + +std::vector GflagsManager::declareGflags(const cpp2::ConfigModule& module) { + std::vector configItems; + if (module == cpp2::ConfigModule::UNKNOWN) { + return configItems; + } + auto configModeMap = parseConfigJson(FLAGS_gflags_mode_json); std::vector flags; gflags::GetAllFlags(&flags); for (auto& flag : flags) { auto& name = flag.name; auto& type = flag.type; cpp2::ConfigType cType; - // default config type will take effect after reboot - cpp2::ConfigMode mode = cpp2::ConfigMode::REBOOT; VariantType value; std::string valueStr; - // TODO: all int32 and uint32 are converted to int64 - if (type == "uint32" || type == "int32" || type == "int64") { + // default config type would be immutable + cpp2::ConfigMode mode = cpp2::ConfigMode::IMMUTABLE; + auto iter = configModeMap.find(name); + if (iter != configModeMap.end()) { + mode = iter->second; + } + // ignore some useless gflags + if (mode == cpp2::ConfigMode::IGNORED) { + continue; + } + if (module == cpp2::ConfigModule::META) { + // all config of meta is immutable for now + mode = cpp2::ConfigMode::IMMUTABLE; + } + + // TODO: all int32/uint32/uint64 gflags are converted to int64 for now + if (type == "uint32" || type == "int32" || type == "int64" || type == "uint64") { cType = cpp2::ConfigType::INT64; value = folly::to(flag.current_value); valueStr = gflagsValueToThriftValue(flag); @@ -57,15 +104,28 @@ void GflagsManager::declareGflags() { continue; } - if (name == "load_data_interval_secs") { - mode = cpp2::ConfigMode::MUTABLE; - } - if (module_ == cpp2::ConfigModule::META) { - // all config of meta is immutable for now - mode = cpp2::ConfigMode::IMMUTABLE; - } + configItems.emplace_back(toThriftConfigItem(module, name, cType, mode, valueStr)); + } + LOG(INFO) << "Prepare to register " << configItems.size() << " gflags to meta"; + return configItems; +} - gflagsDeclared_.emplace_back(toThriftConfigItem(module_, name, cType, mode, valueStr)); +void GflagsManager::getGflagsModule(cpp2::ConfigModule& gflagsModule) { + // get current process according to gflags pid_file + gflags::CommandLineFlagInfo pid; + if (gflags::GetCommandLineFlagInfo("pid_file", &pid)) { + auto defaultPid = pid.default_value; + if (defaultPid.find("nebula-graphd") != std::string::npos) { + gflagsModule = cpp2::ConfigModule::GRAPH; + } else if (defaultPid.find("nebula-storaged") != std::string::npos) { + gflagsModule = cpp2::ConfigModule::STORAGE; + } else if (defaultPid.find("nebula-metad") != std::string::npos) { + gflagsModule = cpp2::ConfigModule::META; + } else { + LOG(ERROR) << "Should not reach here"; + } + } else { + LOG(INFO) << "Unknown config module"; } } diff --git a/src/meta/GflagsManager.h b/src/meta/GflagsManager.h index 038f627cb84..0d14b668b72 100644 --- a/src/meta/GflagsManager.h +++ b/src/meta/GflagsManager.h @@ -29,28 +29,24 @@ class GflagsManager { virtual folly::Future>> listConfigs(const cpp2::ConfigModule& module) = 0; - virtual folly::Future> registerConfig(const cpp2::ConfigModule& module, - const std::string& name, - const cpp2::ConfigType& type, - const cpp2::ConfigMode& mode, - const std::string& defaultValue) = 0; + static void getGflagsModule(cpp2::ConfigModule& gflagsModule); - virtual Status init() = 0; + static std::vector declareGflags(const cpp2::ConfigModule& module); protected: virtual ~GflagsManager() = default; - void declareGflags(); + static std::unordered_map + parseConfigJson(const std::string& json); template - std::string gflagsValueToThriftValue(const gflags::CommandLineFlagInfo& flag); - - cpp2::ConfigModule module_{cpp2::ConfigModule::UNKNOWN}; - std::vector gflagsDeclared_; + static std::string gflagsValueToThriftValue(const gflags::CommandLineFlagInfo& flag); }; std::string toThriftValueStr(const cpp2::ConfigType& type, const VariantType& value); +cpp2::ConfigMode toThriftConfigMode(const std::string& key); + cpp2::ConfigItem toThriftConfigItem(const cpp2::ConfigModule& module, const std::string& name, const cpp2::ConfigType& type, const cpp2::ConfigMode& mode, const std::string& value); diff --git a/src/meta/KVBasedGflagsManager.cpp b/src/meta/KVBasedGflagsManager.cpp index 17adaea7332..fd367c47291 100644 --- a/src/meta/KVBasedGflagsManager.cpp +++ b/src/meta/KVBasedGflagsManager.cpp @@ -21,9 +21,10 @@ KVBasedGflagsManager::~KVBasedGflagsManager() { } Status KVBasedGflagsManager::init() { - getGflagsModule(); - declareGflags(); - return registerGflags(); + auto gflagsModule = cpp2::ConfigModule::UNKNOWN; + getGflagsModule(gflagsModule); + gflagsDeclared_ = declareGflags(gflagsModule); + return registerGflags(gflagsDeclared_); } folly::Future> @@ -48,41 +49,13 @@ KVBasedGflagsManager::listConfigs(const cpp2::ConfigModule& module) { return Status::NotSupported(); } -folly::Future> -KVBasedGflagsManager::registerConfig(const cpp2::ConfigModule& module, const std::string& name, - const cpp2::ConfigType& type, const cpp2::ConfigMode& mode, - const std::string& value) { - UNUSED(module); UNUSED(name); UNUSED(type); UNUSED(mode); UNUSED(value); - LOG(FATAL) << "Unimplement!"; - return Status::NotSupported(); -} - -void KVBasedGflagsManager::getGflagsModule() { - // get current process according to gflags pid_file - gflags::CommandLineFlagInfo pid; - if (gflags::GetCommandLineFlagInfo("pid_file", &pid)) { - auto defaultPid = pid.default_value; - if (defaultPid.find("nebula-graphd") != std::string::npos) { - module_ = cpp2::ConfigModule::GRAPH; - } else if (defaultPid.find("nebula-storaged") != std::string::npos) { - module_ = cpp2::ConfigModule::STORAGE; - } else if (defaultPid.find("nebula-metad") != std::string::npos) { - module_ = cpp2::ConfigModule::META; - } else { - LOG(ERROR) << "Should not reach here"; - } - } else { - LOG(ERROR) << "Should not reach here"; - } -} - -Status KVBasedGflagsManager::registerGflags() { +Status KVBasedGflagsManager::registerGflags(const std::vector& gflagsDeclared) { // based on kv, so we will retry until register config successfully bool succeeded = false; while (!succeeded) { std::vector data; folly::SharedMutex::WriteHolder wHolder(LockUtils::configLock()); - for (const auto& item : gflagsDeclared_) { + for (const auto& item : gflagsDeclared) { auto module = item.get_module(); auto name = item.get_name(); auto type = item.get_type(); diff --git a/src/meta/KVBasedGflagsManager.h b/src/meta/KVBasedGflagsManager.h index f919a4672b4..9702b1595a7 100644 --- a/src/meta/KVBasedGflagsManager.h +++ b/src/meta/KVBasedGflagsManager.h @@ -32,18 +32,12 @@ class KVBasedGflagsManager : public GflagsManager { folly::Future>> listConfigs(const cpp2::ConfigModule& module) override; - folly::Future> registerConfig(const cpp2::ConfigModule& module, - const std::string& name, - const cpp2::ConfigType& type, - const cpp2::ConfigMode& mode, - const std::string& defaultValue) override; - - Status init() override; + Status init(); private: - Status registerGflags(); + Status registerGflags(const std::vector& gflagsDeclared); - void getGflagsModule(); + std::vector gflagsDeclared_; kvstore::NebulaStore* kvstore_ = nullptr; }; diff --git a/src/meta/client/MetaClient.cpp b/src/meta/client/MetaClient.cpp index ef5a172d615..6219338f55b 100644 --- a/src/meta/client/MetaClient.cpp +++ b/src/meta/client/MetaClient.cpp @@ -9,12 +9,14 @@ #include "network/NetworkUtils.h" #include "meta/NebulaSchemaProvider.h" #include "meta/ClusterIdMan.h" +#include "meta/GflagsManager.h" DEFINE_int32(load_data_interval_secs, 2 * 60, "Load data interval"); DEFINE_int32(heartbeat_interval_secs, 10, "Heartbeat interval"); DEFINE_int32(meta_client_retry_times, 3, "meta client retry times, 0 means no retry"); DEFINE_int32(meta_client_retry_interval_secs, 1, "meta client sleep interval between retry"); DEFINE_string(cluster_id_path, "cluster.id", "file path saved clusterId"); +DECLARE_string(gflags_mode_json); namespace nebula { namespace meta { @@ -63,7 +65,9 @@ bool MetaClient::isMetadReady() { } bool MetaClient::waitForMetadReady(int count, int retryIntervalSecs) { - setGflagsModule(); + std::string gflagsJsonPath; + GflagsManager::getGflagsModule(gflagsModule_); + gflagsDeclared_ = GflagsManager::declareGflags(gflagsModule_); isRunning_ = true; int tryCount = count; while (!isMetadReady() && ((count == -1) || (tryCount > 0)) && isRunning_) { @@ -1156,6 +1160,9 @@ MetaClient::regConfig(const std::vector& items) { folly::Future>> MetaClient::getConfig(const cpp2::ConfigModule& module, const std::string& name) { + if (!configReady_) { + return Status::Error("Not ready!"); + } cpp2::ConfigItem item; item.set_module(module); item.set_name(name); @@ -1174,6 +1181,9 @@ MetaClient::getConfig(const cpp2::ConfigModule& module, const std::string& name) folly::Future> MetaClient::setConfig(const cpp2::ConfigModule& module, const std::string& name, const cpp2::ConfigType& type, const std::string& value) { + if (!configReady_) { + return Status::Error("Not ready!"); + } cpp2::ConfigItem item; item.set_module(module); item.set_name(name); @@ -1195,6 +1205,9 @@ MetaClient::setConfig(const cpp2::ConfigModule& module, const std::string& name, folly::Future>> MetaClient::listConfigs(const cpp2::ConfigModule& module) { + if (!configReady_) { + return Status::Error("Not ready!"); + } cpp2::ListConfigsReq req; req.set_module(module); folly::Promise>> promise; @@ -1207,36 +1220,24 @@ MetaClient::listConfigs(const cpp2::ConfigModule& module) { return future; } -void MetaClient::setGflagsModule(const cpp2::ConfigModule& module) { - if (module != cpp2::ConfigModule::UNKNOWN) { - gflagsModule_ = module; - return; - } - - // get current process according to gflags pid_file - gflags::CommandLineFlagInfo pid; - if (gflags::GetCommandLineFlagInfo("pid_file", &pid)) { - auto defaultPid = pid.default_value; - if (defaultPid.find("nebula-graphd") != std::string::npos) { - gflagsModule_ = cpp2::ConfigModule::GRAPH; - } else if (defaultPid.find("nebula-storaged") != std::string::npos) { - gflagsModule_ = cpp2::ConfigModule::STORAGE; - } else if (defaultPid.find("nebula-metad") != std::string::npos) { - gflagsModule_ = cpp2::ConfigModule::META; - } else { - LOG(ERROR) << "Should not reach here"; - } - } else { - LOG(ERROR) << "Should not reach here"; - } -} - void MetaClient::loadCfgThreadFunc() { loadCfg(); addLoadCfgTask(); } +bool MetaClient::registerCfg() { + auto ret = regConfig(gflagsDeclared_).get(); + if (ret.ok()) { + LOG(INFO) << "Register gflags ok " << gflagsDeclared_.size(); + configReady_ = true; + } + return configReady_; +} + void MetaClient::loadCfg() { + if (!configReady_ && !registerCfg()) { + return; + } // only load current module's config is enough auto ret = listConfigs(gflagsModule_).get(); if (ret.ok()) { diff --git a/src/meta/client/MetaClient.h b/src/meta/client/MetaClient.h index a641fb366e7..08c41b0f317 100644 --- a/src/meta/client/MetaClient.h +++ b/src/meta/client/MetaClient.h @@ -10,12 +10,14 @@ #include "base/Base.h" #include #include +#include #include "gen-cpp2/MetaServiceAsyncClient.h" #include "base/Status.h" #include "base/StatusOr.h" #include "thread/GenericWorker.h" #include "thrift/ThriftClientManager.h" #include "meta/SchemaProviderIf.h" +#include "meta/GflagsManager.h" DECLARE_int32(meta_client_retry_times); @@ -85,6 +87,9 @@ class MetaChangedListener { }; class MetaClient { + FRIEND_TEST(ConfigManTest, MetaConfigManTest); + FRIEND_TEST(ConfigManTest, MockConfigTest); + public: explicit MetaClient(std::shared_ptr ioThreadPool, std::vector addrs, @@ -216,10 +221,6 @@ class MetaClient { folly::Future>> listConfigs(const cpp2::ConfigModule& module); - cpp2::ConfigModule& getGflagsModule() {return gflagsModule_;} - - void setGflagsModule(const cpp2::ConfigModule& module = cpp2::ConfigModule::UNKNOWN); - // Opeartions for cache. StatusOr getSpaceIdByNameFromCache(const std::string& name); @@ -263,6 +264,7 @@ class MetaClient { void loadCfgThreadFunc(); void loadCfg(); + bool registerCfg(); void addLoadCfgTask(); void updateGflagsValue(const ConfigItem& item); @@ -349,6 +351,8 @@ class MetaClient { MetaConfigMap metaConfigMap_; folly::RWSpinLock configCacheLock_; cpp2::ConfigModule gflagsModule_{cpp2::ConfigModule::UNKNOWN}; + std::atomic_bool configReady_{false}; + std::vector gflagsDeclared_; }; } // namespace meta diff --git a/src/meta/test/CMakeLists.txt b/src/meta/test/CMakeLists.txt index 0dd8f9fd4d9..20c4f923c3a 100644 --- a/src/meta/test/CMakeLists.txt +++ b/src/meta/test/CMakeLists.txt @@ -42,6 +42,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -72,6 +73,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -103,6 +105,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -165,6 +168,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -192,7 +196,7 @@ nebula_add_test( $ $ $ - $ + $ $ $ $ @@ -201,6 +205,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES proxygenhttpserver proxygenlib @@ -240,6 +245,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES proxygenhttpserver proxygenlib @@ -279,6 +285,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES proxygenhttpserver proxygenlib @@ -309,6 +316,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -342,6 +350,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -373,6 +382,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -403,6 +413,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} @@ -431,6 +442,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} diff --git a/src/meta/test/ConfigManTest.cpp b/src/meta/test/ConfigManTest.cpp index e82b5c29218..1d0ca5c3e2b 100644 --- a/src/meta/test/ConfigManTest.cpp +++ b/src/meta/test/ConfigManTest.cpp @@ -7,6 +7,7 @@ #include "base/Base.h" #include #include +#include "base/Configuration.h" #include "fs/TempDir.h" #include "meta/test/TestUtils.h" #include "meta/GflagsManager.h" @@ -218,30 +219,30 @@ TEST(ConfigManTest, MetaConfigManTest) { auto client = std::make_shared(threadPool, std::vector{HostAddr(localIp, sc->port_)}); client->waitForMetadReady(); - client->setGflagsModule(module); + client->gflagsModule_ = module; ClientBasedGflagsManager cfgMan(client.get()); - cfgMan.module_ = module; // mock some test gflags to meta { auto mode = meta::cpp2::ConfigMode::MUTABLE; - cfgMan.gflagsDeclared_.emplace_back(toThriftConfigItem( + std::vector configItems; + configItems.emplace_back(toThriftConfigItem( module, "int64_key_immutable", cpp2::ConfigType::INT64, cpp2::ConfigMode::IMMUTABLE, toThriftValueStr(cpp2::ConfigType::INT64, 100L))); - cfgMan.gflagsDeclared_.emplace_back(toThriftConfigItem( + configItems.emplace_back(toThriftConfigItem( module, "int64_key", cpp2::ConfigType::INT64, mode, toThriftValueStr(cpp2::ConfigType::INT64, 101L))); - cfgMan.gflagsDeclared_.emplace_back(toThriftConfigItem( + configItems.emplace_back(toThriftConfigItem( module, "bool_key", cpp2::ConfigType::BOOL, mode, toThriftValueStr(cpp2::ConfigType::BOOL, false))); - cfgMan.gflagsDeclared_.emplace_back(toThriftConfigItem( + configItems.emplace_back(toThriftConfigItem( module, "double_key", cpp2::ConfigType::DOUBLE, mode, toThriftValueStr(cpp2::ConfigType::DOUBLE, 1.23))); std::string defaultValue = "something"; - cfgMan.gflagsDeclared_.emplace_back(toThriftConfigItem( + configItems.emplace_back(toThriftConfigItem( module, "string_key", cpp2::ConfigType::STRING, mode, toThriftValueStr(cpp2::ConfigType::STRING, defaultValue))); - cfgMan.registerGflags(); + cfgMan.registerGflags(configItems); } // try to set/get config not registered @@ -381,20 +382,20 @@ TEST(ConfigManTest, MockConfigTest) { auto client = std::make_shared(threadPool, std::vector{HostAddr(localIp, sc->port_)}); client->waitForMetadReady(); - client->setGflagsModule(module); + client->gflagsModule_ = module; ClientBasedGflagsManager clientCfgMan(client.get()); - clientCfgMan.module_ = module; + std::vector configItems; for (int i = 0; i < 5; i++) { std::string name = "test" + std::to_string(i); std::string value = "v" + std::to_string(i); - clientCfgMan.gflagsDeclared_.emplace_back( - toThriftConfigItem(module, name, type, mode, value)); + configItems.emplace_back(toThriftConfigItem(module, name, type, mode, value)); } - clientCfgMan.registerGflags(); + clientCfgMan.registerGflags(configItems); auto consoleClient = std::make_shared(threadPool, std::vector{HostAddr(localIp, sc->port_)}); + consoleClient->waitForMetadReady(); ClientBasedGflagsManager console(consoleClient.get()); // update in console for (int i = 0; i < 5; i++) { diff --git a/src/storage/StorageServer.cpp b/src/storage/StorageServer.cpp index b8dfaf73a2d..8e6e1d75ea9 100644 --- a/src/storage/StorageServer.cpp +++ b/src/storage/StorageServer.cpp @@ -107,7 +107,6 @@ bool StorageServer::start() { } gFlagsMan_ = std::make_unique(metaClient_.get()); - gFlagsMan_->init(); LOG(INFO) << "Init schema manager"; schemaMan_ = meta::SchemaManager::create(); diff --git a/src/storage/test/CMakeLists.txt b/src/storage/test/CMakeLists.txt index 287cc7763b0..d79ac42a23b 100644 --- a/src/storage/test/CMakeLists.txt +++ b/src/storage/test/CMakeLists.txt @@ -17,6 +17,7 @@ set(storage_test_deps $ $ $ + $ ) @@ -221,6 +222,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES} diff --git a/src/tools/storage-perf/CMakeLists.txt b/src/tools/storage-perf/CMakeLists.txt index d32a1873962..0705b7c1fdb 100644 --- a/src/tools/storage-perf/CMakeLists.txt +++ b/src/tools/storage-perf/CMakeLists.txt @@ -23,6 +23,7 @@ nebula_add_executable( $ $ $ + $ LIBRARIES ${ROCKSDB_LIBRARIES} ${THRIFT_LIBRARIES}