Modify config (vesoft-inc#65)

* Add config * modify test * rebase upstream * rebase upstream
bright-starry-sky · Aug 17, 2020 · ffe94df · ffe94df
1 parent e4b764d
commit ffe94df
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 236 deletions.
diff --git a/src/meta/CMakeLists.txt b/src/meta/CMakeLists.txt
@@ -86,19 +86,4 @@ nebula_add_library(
     MetaHttpReplaceHostHandler.cpp
 )
 
-#nebula_add_library(
-#    meta_client OBJECT
-#    client/MetaClient.cpp
-#)
-
-#nebula_add_library(
-#    gflags_man_obj OBJECT
-#    GflagsManager.cpp
-#)
-
-#nebula_add_library(
-#    meta_gflags_man_obj OBJECT
-#    ClientBasedGflagsManager.cpp
-#)
-
 nebula_add_subdirectory(test)
diff --git a/src/meta/MetaServiceUtils.cpp b/src/meta/MetaServiceUtils.cpp
@@ -723,14 +723,13 @@ std::string MetaServiceUtils::configKeyPrefix(const cpp2::ConfigModule& module)
     return key;
 }
 
-std::string MetaServiceUtils::configValue(const cpp2::ConfigType& valueType,
-                                          const cpp2::ConfigMode& valueMode,
-                                          const std::string& config) {
-    std::string val;
-    val.reserve(sizeof(cpp2::ConfigType) + sizeof(cpp2::ConfigMode) + config.size());
-    val.append(reinterpret_cast<const char*>(&valueType), sizeof(cpp2::ConfigType))
-       .append(reinterpret_cast<const char*>(&valueMode), sizeof(cpp2::ConfigMode))
-       .append(config);
+std::string MetaServiceUtils::configValue(const cpp2::ConfigMode& valueMode,
+                                          const Value& value) {
+    std::string val, cVal;
+    apache::thrift::CompactSerializer::serialize(value, &cVal);
+    val.reserve(sizeof(cpp2::ConfigMode) + cVal.size());
+    val.append(reinterpret_cast<const char*>(&valueMode), sizeof(cpp2::ConfigMode))
+       .append(cVal);
     return val;
 }
 
@@ -747,16 +746,15 @@ ConfigName MetaServiceUtils::parseConfigKey(folly::StringPiece rawKey) {
 
 cpp2::ConfigItem MetaServiceUtils::parseConfigValue(folly::StringPiece rawData) {
     int32_t offset = 0;
-    cpp2::ConfigType type = *reinterpret_cast<const cpp2::ConfigType*>(rawData.data() + offset);
-    offset += sizeof(cpp2::ConfigType);
     cpp2::ConfigMode mode = *reinterpret_cast<const cpp2::ConfigMode*>(rawData.data() + offset);
     offset += sizeof(cpp2::ConfigMode);
-    auto value = rawData.subpiece(offset, rawData.size() - offset);
+    Value value;
+    apache::thrift::CompactSerializer::deserialize(
+            rawData.subpiece(offset, rawData.size() - offset), value);
 
     cpp2::ConfigItem item;
-    item.set_type(type);
     item.set_mode(mode);
-    item.set_value(value.str());
+    item.set_value(value);
     return item;
 }
 

diff --git a/src/meta/MetaServiceUtils.h b/src/meta/MetaServiceUtils.h
@@ -204,9 +204,8 @@ class MetaServiceUtils final {
 
     static std::string configKeyPrefix(const cpp2::ConfigModule& module);
 
-    static std::string configValue(const cpp2::ConfigType& valueType,
-                                   const cpp2::ConfigMode& valueMode,
-                                   const std::string& config);
+    static std::string configValue(const cpp2::ConfigMode& valueMode,
+                                   const Value& config);
 
     static ConfigName parseConfigKey(folly::StringPiece rawData);
 

diff --git a/src/meta/processors/configMan/RegConfigProcessor.cpp b/src/meta/processors/configMan/RegConfigProcessor.cpp
@@ -16,16 +16,19 @@ void RegConfigProcessor::process(const cpp2::RegConfigReq& req) {
         for (const auto& item : req.get_items()) {
             auto module = item.get_module();
             auto name = item.get_name();
-            auto type = item.get_type();
             auto mode = item.get_mode();
             auto value = item.get_value();
+            VLOG(1) << "Config name: " << name
+                    << ", mode: " << meta::cpp2::_ConfigMode_VALUES_TO_NAMES.at(mode)
+                    << ", module: " << meta::cpp2::_ConfigModule_VALUES_TO_NAMES.at(module)
+                    << ", value: " << value;
 
             std::string configKey = MetaServiceUtils::configKey(module, name);
             // ignore config which has been registered before
             if (doGet(configKey).ok()) {
                 continue;
             }
-            std::string configValue = MetaServiceUtils::configValue(type, mode, value);
+            std::string configValue = MetaServiceUtils::configValue(mode, value);
             data.emplace_back(std::move(configKey), std::move(configValue));
         }
 

diff --git a/src/meta/processors/configMan/SetConfigProcessor.cpp b/src/meta/processors/configMan/SetConfigProcessor.cpp
@@ -9,82 +9,31 @@
 
 namespace nebula {
 namespace meta {
-
-std::unordered_set<std::string> SetConfigProcessor::mutableFields_ = {
-    // rocksdb_column_family_options
-    "disable_auto_compactions",
-    // TODO: write_buffer_size will cause rocksdb crash
-    "write_buffer_size",
-    "max_write_buffer_number",
-    "level0_file_num_compaction_trigger",
-    "level0_slowdown_writes_trigger",
-    "level0_stop_writes_trigger",
-    "target_file_size_base",
-    "target_file_size_multiplier",
-    "max_bytes_for_level_base",
-    "max_bytes_for_level_multiplier",
-
-    // rocksdb_db_options
-    "max_total_wal_size",
-    "delete_obsolete_files_period_micros",
-    "max_background_jobs",
-    "stats_dump_period_sec",
-    "compaction_readahead_size",
-    "writable_file_max_buffer_size",
-    "bytes_per_sync",
-    "wal_bytes_per_sync",
-    "delayed_write_rate",
-    "avoid_flush_during_shutdown",
-    "max_open_files"
-};
-
 void SetConfigProcessor::process(const cpp2::SetConfigReq& req) {
     std::vector<kvstore::KV> data;
     auto module = req.get_item().get_module();
     auto name = req.get_item().get_name();
-    auto type = req.get_item().get_type();
     auto value = req.get_item().get_value();
 
     folly::SharedMutex::WriteHolder wHolder(LockUtils::configLock());
     cpp2::ErrorCode code = cpp2::ErrorCode::SUCCEEDED;
-
     do {
-        if (type != cpp2::ConfigType::NESTED) {
-            if (module != cpp2::ConfigModule::ALL) {
-                // When we set config of a specified module, check if it exists.
-                // If it exists and is mutable, update it.
-                code = setOneConfig(module, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
-            } else {
-                // When we set config of all module, then try to set it of every module.
-                code = setOneConfig(cpp2::ConfigModule::GRAPH, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
-                code = setOneConfig(cpp2::ConfigModule::STORAGE, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
+        if (module != cpp2::ConfigModule::ALL) {
+            // When we set config of a specified module, check if it exists.
+            // If it exists and is mutable, update it.
+            code = setConfig(module, name, value, data);
+            if (code != cpp2::ErrorCode::SUCCEEDED) {
+                break;
             }
         } else {
-            // For those nested options like FLAGS_rocksdb_db_options, if any field has changed,
-            // we update them and put it back
-            if (module != cpp2::ConfigModule::ALL) {
-                code = setNestedConfig(module, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
-            } else {
-                code = setNestedConfig(cpp2::ConfigModule::GRAPH, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
-                code = setNestedConfig(cpp2::ConfigModule::STORAGE, name, type, value, data);
-                if (code != cpp2::ErrorCode::SUCCEEDED) {
-                    break;
-                }
+            // When we set config of all module, then try to set it of every module.
+            code = setConfig(cpp2::ConfigModule::GRAPH, name, value, data);
+            if (code != cpp2::ErrorCode::SUCCEEDED) {
+                break;
+            }
+            code = setConfig(cpp2::ConfigModule::STORAGE, name, value, data);
+            if (code != cpp2::ErrorCode::SUCCEEDED) {
+                break;
             }
         }
 
@@ -98,11 +47,10 @@ void SetConfigProcessor::process(const cpp2::SetConfigReq& req) {
     onFinished();
 }
 
-cpp2::ErrorCode SetConfigProcessor::setOneConfig(const cpp2::ConfigModule& module,
-                                                 const std::string& name,
-                                                 const cpp2::ConfigType& type,
-                                                 const std::string& value,
-                                                 std::vector<kvstore::KV>& data) {
+cpp2::ErrorCode SetConfigProcessor::setConfig(const cpp2::ConfigModule& module,
+                                              const std::string& name,
+                                              const Value& value,
+                                              std::vector<kvstore::KV>& data) {
     std::string configKey = MetaServiceUtils::configKey(module, name);
     auto ret = doGet(std::move(configKey));
     if (!ret.ok()) {
@@ -114,56 +62,10 @@ cpp2::ErrorCode SetConfigProcessor::setOneConfig(const cpp2::ConfigModule& modul
     if (curMode == cpp2::ConfigMode::IMMUTABLE) {
         return cpp2::ErrorCode::E_CONFIG_IMMUTABLE;
     }
-    std::string configValue = MetaServiceUtils::configValue(type, curMode, value);
+    std::string configValue = MetaServiceUtils::configValue(curMode, value);
     data.emplace_back(std::move(configKey), std::move(configValue));
     return cpp2::ErrorCode::SUCCEEDED;
 }
 
-cpp2::ErrorCode SetConfigProcessor::setNestedConfig(const cpp2::ConfigModule& module,
-                                                    const std::string& name,
-                                                    const cpp2::ConfigType& type,
-                                                    const std::string& updateList,
-                                                    std::vector<kvstore::KV>& data) {
-    std::string configKey = MetaServiceUtils::configKey(module, name);
-    auto ret = doGet(std::move(configKey));
-    if (!ret.ok()) {
-        return cpp2::ErrorCode::E_NOT_FOUND;
-    }
-
-    cpp2::ConfigItem item = MetaServiceUtils::parseConfigValue(ret.value());
-    cpp2::ConfigMode curMode = item.get_mode();
-    if (curMode == cpp2::ConfigMode::IMMUTABLE) {
-        return cpp2::ErrorCode::E_CONFIG_IMMUTABLE;
-    }
-
-    conf::Configuration conf;
-    std::vector<std::string> updateFields;
-    folly::split(",", updateList, updateFields, true);
-    bool updated = false;
-    for (const auto& field : updateFields) {
-        auto pos = field.find("=");
-        if (pos == std::string::npos) {
-            LOG(ERROR) << "Should not reach here";
-            continue;
-        }
-        auto key = field.substr(0, pos);
-        auto value = field.substr(pos + 1);
-        // TODO: Maybe need to handle illegal value here
-        if (!conf.upsertStringField(key.c_str(), value).ok()) {
-            LOG(ERROR) << "Update configs failed: " << key;
-            return cpp2::ErrorCode::E_UNSUPPORTED;
-        }
-        if (mutableFields_.count(key)) {
-            updated = true;
-        }
-    }
-
-    if (updated) {
-        std::string configValue = MetaServiceUtils::configValue(type, curMode, conf.dumpToString());
-        data.emplace_back(std::move(configKey), std::move(configValue));
-    }
-    return cpp2::ErrorCode::SUCCEEDED;
-}
-
 }  // namespace meta
 }  // namespace nebula
diff --git a/src/meta/processors/configMan/SetConfigProcessor.h b/src/meta/processors/configMan/SetConfigProcessor.h
@@ -20,13 +20,10 @@ class SetConfigProcessor : public BaseProcessor<cpp2::ExecResp> {
 
     void process(const cpp2::SetConfigReq& req);
 
-    cpp2::ErrorCode setOneConfig(const cpp2::ConfigModule& module, const std::string& name,
-                                 const cpp2::ConfigType& type, const std::string& value,
-                                 std::vector<kvstore::KV>& data);
-
-    cpp2::ErrorCode setNestedConfig(const cpp2::ConfigModule& module, const std::string& name,
-                                    const cpp2::ConfigType& type, const std::string& value,
-                                    std::vector<kvstore::KV>& data);
+    cpp2::ErrorCode setConfig(const cpp2::ConfigModule& module,
+                              const std::string& name,
+                              const Value& value,
+                              std::vector<kvstore::KV>& data);
 
 private:
     explicit SetConfigProcessor(kvstore::KVStore* kvstore)

diff --git a/src/meta/test/CMakeLists.txt b/src/meta/test/CMakeLists.txt
@@ -96,21 +96,6 @@ nebula_add_test(
         gtest
 )
 
-#nebula_add_test(
-#    NAME
-#        config_man_test
-#    SOURCES
-#        ConfigManTest.cpp
-#    OBJECTS
-#        ${meta_test_deps}
-#    LIBRARIES
-#        ${ROCKSDB_LIBRARIES}
-#        ${THRIFT_LIBRARIES}
-#        wangle
-#        gtest
-#)
-
-
 nebula_add_test(
     NAME
         active_hosts_man_test