diff --git a/src/meta/CMakeLists.txt b/src/meta/CMakeLists.txt index 2fbb78726..7f7ce8eb3 100644 --- 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 index f5ce65afa..899ea7421 100644 --- 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(&valueType), sizeof(cpp2::ConfigType)) - .append(reinterpret_cast(&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(&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(rawData.data() + offset); - offset += sizeof(cpp2::ConfigType); cpp2::ConfigMode mode = *reinterpret_cast(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 index 33428c47b..be47d68c8 100644 --- 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 index 0d4a4f1e2..8a14a2908 100644 --- 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 index fc5b93765..159358416 100644 --- 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 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 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& data) { +cpp2::ErrorCode SetConfigProcessor::setConfig(const cpp2::ConfigModule& module, + const std::string& name, + const Value& value, + std::vector& 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& 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 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 index 40bb0675a..bce62b136 100644 --- a/src/meta/processors/configMan/SetConfigProcessor.h +++ b/src/meta/processors/configMan/SetConfigProcessor.h @@ -20,13 +20,10 @@ class SetConfigProcessor : public BaseProcessor { 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& data); - - cpp2::ErrorCode setNestedConfig(const cpp2::ConfigModule& module, const std::string& name, - const cpp2::ConfigType& type, const std::string& value, - std::vector& data); + cpp2::ErrorCode setConfig(const cpp2::ConfigModule& module, + const std::string& name, + const Value& value, + std::vector& data); private: explicit SetConfigProcessor(kvstore::KVStore* kvstore) diff --git a/src/meta/test/CMakeLists.txt b/src/meta/test/CMakeLists.txt index 20b52e3d8..3798708d6 100644 --- 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 diff --git a/src/meta/test/MetaClientTest.cpp b/src/meta/test/MetaClientTest.cpp index fb2e64575..a5abc5a15 100644 --- a/src/meta/test/MetaClientTest.cpp +++ b/src/meta/test/MetaClientTest.cpp @@ -10,7 +10,7 @@ #include "common/network/NetworkUtils.h" #include "common/meta/ServerBasedSchemaManager.h" #include "common/meta/GflagsManager.h" -#include "common/meta/ClientBasedGflagsManager.h" +#include "common/conf/Configuration.h" #include #include #include "meta/test/TestUtils.h" @@ -1184,6 +1184,18 @@ TEST(MetaClientTest, RetryUntilLimitTest) { } } +cpp2::ConfigItem initConfigItem(cpp2::ConfigModule module, + std::string name, + cpp2::ConfigMode mode, + Value value) { + cpp2::ConfigItem configItem; + configItem.set_module(module); + configItem.set_name(name); + configItem.set_mode(mode); + configItem.set_value(value); + return configItem; +} + TEST(MetaClientTest, Config) { FLAGS_heartbeat_interval_secs = 1; fs::TempDir rootPath("/tmp/MetaClientTest.Config.XXXXXX"); @@ -1217,17 +1229,11 @@ TEST(MetaClientTest, Config) { } // Set { - auto resp = client->setConfig(cpp2::ConfigModule::GRAPH, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + auto resp = client->setConfig(cpp2::ConfigModule::GRAPH, "minloglevel", 3).get(); EXPECT_TRUE(!resp.ok()); - resp = client->setConfig(cpp2::ConfigModule::META, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + resp = client->setConfig(cpp2::ConfigModule::META, "minloglevel", 3).get(); EXPECT_TRUE(!resp.ok()); - resp = client->setConfig(cpp2::ConfigModule::STORAGE, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + resp = client->setConfig(cpp2::ConfigModule::STORAGE, "minloglevel", 3).get(); EXPECT_TRUE(!resp.ok()); } // Get @@ -1242,23 +1248,15 @@ TEST(MetaClientTest, Config) { EXPECT_TRUE(!resp.ok()); } - auto item1 = toThriftConfigItem( - cpp2::ConfigModule::GRAPH, "minloglevel", cpp2::ConfigType::INT64, - cpp2::ConfigMode::MUTABLE, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(2))); - auto item2 = toThriftConfigItem( - cpp2::ConfigModule::META, "minloglevel", cpp2::ConfigType::INT64, - cpp2::ConfigMode::MUTABLE, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(2))); - auto item3 = toThriftConfigItem( - cpp2::ConfigModule::STORAGE, "minloglevel", cpp2::ConfigType::INT64, - cpp2::ConfigMode::MUTABLE, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(2))); // Reg { - std::vector configItems { - item1, item2, item3, - }; + std::vector configItems; + configItems.emplace_back(initConfigItem( + cpp2::ConfigModule::GRAPH, "minloglevel", cpp2::ConfigMode::MUTABLE, 2)); + configItems.emplace_back(initConfigItem( + cpp2::ConfigModule::META, "minloglevel", cpp2::ConfigMode::MUTABLE, 2)); + configItems.emplace_back(initConfigItem( + cpp2::ConfigModule::STORAGE, "minloglevel", cpp2::ConfigMode::MUTABLE, 2)); auto resp = client->regConfig(configItems).get(); ASSERT(resp.ok()); } @@ -1279,44 +1277,34 @@ TEST(MetaClientTest, Config) { } // Set { - auto resp = client->setConfig(cpp2::ConfigModule::GRAPH, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + auto resp = client->setConfig(cpp2::ConfigModule::GRAPH, "minloglevel", 3).get(); EXPECT_TRUE(resp.ok()); - resp = client->setConfig(cpp2::ConfigModule::META, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + resp = client->setConfig(cpp2::ConfigModule::META, "minloglevel", 3).get(); EXPECT_TRUE(resp.ok()); - resp = client->setConfig(cpp2::ConfigModule::STORAGE, "minloglevel", - cpp2::ConfigType::INT64, - toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))).get(); + resp = client->setConfig(cpp2::ConfigModule::STORAGE, "minloglevel", 3).get(); EXPECT_TRUE(resp.ok()); } // Get { - item1.set_value(toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))); auto resp = client->getConfig(cpp2::ConfigModule::GRAPH, "minloglevel").get(); EXPECT_TRUE(resp.ok()); - auto config = std::move(resp).value(); - EXPECT_EQ(item1, config[0]); + auto configs = std::move(resp).value(); + EXPECT_EQ(configs[0].get_value(), Value(3)); - item2.set_value(toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))); resp = client->getConfig(cpp2::ConfigModule::META, "minloglevel").get(); EXPECT_TRUE(resp.ok()); - config = std::move(resp).value(); - EXPECT_EQ(item2, config[0]); + configs = std::move(resp).value(); + EXPECT_EQ(configs[0].get_value(), Value(3)); - item3.set_value(toThriftValueStr(cpp2::ConfigType::INT64, static_cast(3))); resp = client->getConfig(cpp2::ConfigModule::STORAGE, "minloglevel").get(); EXPECT_TRUE(resp.ok()); - config = std::move(resp).value(); - EXPECT_EQ(item3, config[0]); + configs = std::move(resp).value(); + EXPECT_EQ(configs[0].get_value(), Value(3)); } // Just avoid memory leak error of clang asan. to waitting asynchronous thread done. sleep(FLAGS_heartbeat_interval_secs * 5); } - TEST(MetaClientTest, RocksdbOptionsTest) { FLAGS_heartbeat_interval_secs = 1; fs::TempDir rootPath("/tmp/RocksdbOptionsTest.XXXXXX"); @@ -1334,24 +1322,31 @@ TEST(MetaClientTest, RocksdbOptionsTest) { auto listener = std::make_unique(); auto module = cpp2::ConfigModule::STORAGE; - auto type = cpp2::ConfigType::NESTED; auto mode = meta::cpp2::ConfigMode::MUTABLE; client->registerListener(listener.get()); client->gflagsModule_ = module; - ClientBasedGflagsManager cfgMan(client); // mock some rocksdb gflags to meta { + auto name = "rocksdb_db_options"; std::vector configItems; - FLAGS_rocksdb_db_options = R"({ - "disable_auto_compactions":"false", - "write_buffer_size":"1048576" - })"; - configItems.emplace_back(toThriftConfigItem( - module, "rocksdb_db_options", type, - mode, toThriftValueStr(type, FLAGS_rocksdb_db_options))); - cfgMan.registerGflags(configItems); + FLAGS_rocksdb_db_options = R"({"disable_auto_compactions":"false","write_buffer_size":"1048576"})"; + Map map; + map.kvs.emplace("disable_auto_compactions", "false"); + map.kvs.emplace("write_buffer_size", "1048576"); + configItems.emplace_back(initConfigItem( + module, name, + mode, Value(map))); + client->regConfig(configItems); + + // get from meta server + auto getRet = client->getConfig(module, name).get(); + ASSERT_TRUE(getRet.ok()); + auto item = getRet.value().front(); + + sleep(FLAGS_heartbeat_interval_secs + 1); + ASSERT_EQ(FLAGS_rocksdb_db_options, GflagsManager::ValueToGflagString(item.get_value())); } { std::vector hosts = {{"0", 0}}; @@ -1362,20 +1357,21 @@ TEST(MetaClientTest, RocksdbOptionsTest) { } { std::string name = "rocksdb_db_options"; - std::string updateValue = "disable_auto_compactions=true," - "level0_file_num_compaction_trigger=4"; + Map map; + map.kvs.emplace("disable_auto_compactions", "true"); + map.kvs.emplace("level0_file_num_compaction_trigger", "4"); + // update config - auto setRet = cfgMan.setConfig(module, name, type, updateValue).get(); + auto setRet = client->setConfig(module, name, Value(map)).get(); ASSERT_TRUE(setRet.ok()); // get from meta server - auto getRet = cfgMan.getConfig(module, name).get(); + auto getRet = client->getConfig(module, name).get(); ASSERT_TRUE(getRet.ok()); auto item = getRet.value().front(); - auto value = boost::get(item.get_value()); sleep(FLAGS_heartbeat_interval_secs + 1); - ASSERT_EQ(FLAGS_rocksdb_db_options, value); + ASSERT_EQ(FLAGS_rocksdb_db_options, GflagsManager::ValueToGflagString(item.get_value())); ASSERT_EQ(listener->options["disable_auto_compactions"], "true"); ASSERT_EQ(listener->options["level0_file_num_compaction_trigger"], "4"); } diff --git a/src/storage/StorageServer.cpp b/src/storage/StorageServer.cpp index aca120a50..122980e02 100644 --- a/src/storage/StorageServer.cpp +++ b/src/storage/StorageServer.cpp @@ -117,8 +117,6 @@ bool StorageServer::start() { return false; } - gFlagsMan_ = std::make_unique(metaClient_.get()); - LOG(INFO) << "Init schema manager"; schemaMan_ = meta::SchemaManager::create(metaClient_.get()); diff --git a/src/storage/StorageServer.h b/src/storage/StorageServer.h index 2d39f7e16..9b3a86d3c 100644 --- a/src/storage/StorageServer.h +++ b/src/storage/StorageServer.h @@ -11,7 +11,6 @@ #include "common/meta/SchemaManager.h" #include "common/meta/IndexManager.h" #include "common/clients/meta/MetaClient.h" -#include "common/meta/ClientBasedGflagsManager.h" #include "common/hdfs/HdfsHelper.h" #include #include "kvstore/NebulaStore.h" @@ -67,7 +66,6 @@ class StorageServer final { std::unique_ptr hdfsHelper_; std::unique_ptr webWorkers_; - std::unique_ptr gFlagsMan_; std::unique_ptr schemaMan_; std::unique_ptr indexMan_; std::unique_ptr env_;