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
vesoft-inc · Aug 27, 2019 · c358d53 · c358d53
1 parent 153545e
commit c358d53
Show file tree

Hide file tree

Showing 24 changed files with 263 additions and 161 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -40,7 +40,6 @@ Status ExecutionEngine::init(std::shared_ptr<folly::IOThreadPoolExecutor> ioExec
     schemaManager_->init(metaClient_.get());
 
     gflagsManager_ = std::make_unique<meta::ClientBasedGflagsManager>(metaClient_.get());
-    gflagsManager_->init();
 
     storage_ = std::make_unique<storage::StorageClient>(ioExecutor, metaClient_.get());
     return Status::OK();

diff --git 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<meta::cpp2::ConfigItem> mockRegisterGflags() {
+    std::vector<meta::cpp2::ConfigItem> 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
@@ -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
@@ -17,6 +17,7 @@ nebula_add_test(
         $<TARGET_OBJECTS:thread_obj>
         $<TARGET_OBJECTS:meta_client>
         $<TARGET_OBJECTS:meta_thrift_obj>
+        $<TARGET_OBJECTS:gflags_man_obj>
     LIBRARIES
         ${ROCKSDB_LIBRARIES}
         ${THRIFT_LIBRARIES}
@@ -44,6 +45,7 @@ nebula_add_test(
         $<TARGET_OBJECTS:meta_client>
         $<TARGET_OBJECTS:meta_thrift_obj>
         $<TARGET_OBJECTS:adHocSchema_obj>
+        $<TARGET_OBJECTS:gflags_man_obj>
     LIBRARIES
         ${ROCKSDB_LIBRARIES}
         ${THRIFT_LIBRARIES}

diff --git a/src/kvstore/test/CMakeLists.txt b/src/kvstore/test/CMakeLists.txt
@@ -13,6 +13,7 @@ set(KVSTORE_TEST_LIBS
     $<TARGET_OBJECTS:network_obj>
     $<TARGET_OBJECTS:thrift_obj>
     $<TARGET_OBJECTS:time_obj>
+    $<TARGET_OBJECTS:gflags_man_obj>
 )
 
 nebula_add_test(

diff --git 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
@@ -18,12 +18,6 @@ ClientBasedGflagsManager::~ClientBasedGflagsManager() {
     metaClient_ = nullptr;
 }
 
-Status ClientBasedGflagsManager::init() {
-    module_ = metaClient_->getGflagsModule();
-    declareGflags();
-    return registerGflags();
-}
-
 template<typename ValueType>
 folly::Future<StatusOr<bool>> 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<StatusOr<bool>>
-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<cpp2::ConfigItem> 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<cpp2::ConfigItem>& 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
@@ -8,17 +8,13 @@
 #define META_CLIENTBASEDGFLAGSMANAGER_H_
 
 #include "base/Base.h"
-#include <gtest/gtest_prod.h>
 #include "meta/GflagsManager.h"
 #include "meta/client/MetaClient.h"
 
 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<StatusOr<std::vector<cpp2::ConfigItem>>>
     listConfigs(const cpp2::ConfigModule& module) override;
 
-    folly::Future<StatusOr<bool>> 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<cpp2::ConfigItem>& items);
 
 private:
-    Status registerGflags();
-
     template<typename ValueType>
     folly::Future<StatusOr<bool>> set(const cpp2::ConfigModule& module, const std::string& name,
                                       const cpp2::ConfigType& type, const ValueType& value);