From f6d218a40709f6605909ffdfc77a0c6a8f7b167b Mon Sep 17 00:00:00 2001 From: zhao liwei Date: Tue, 22 Dec 2020 08:40:58 +0800 Subject: [PATCH] feat: support to modify configs without restart (#682) --- include/dsn/utility/flags.h | 22 ++++++ include/dsn/utility/string_conv.h | 5 ++ include/dsn/utility/utils.h | 24 ++++++- src/http/builtin_http_calls.cpp | 5 ++ src/http/builtin_http_calls.h | 2 + src/http/config_http_service.cpp | 40 +++++++++++ src/utils/flags.cpp | 78 +++++++++++++++++++++ src/utils/test/flag_test.cpp | 105 ++++++++++++++++++++++++++++ src/utils/test/main.cpp | 3 + src/utils/test/string_conv_test.cpp | 46 ++++++++++++ 10 files changed, 328 insertions(+), 2 deletions(-) create mode 100644 src/http/config_http_service.cpp create mode 100644 src/utils/test/flag_test.cpp diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index c312494dfa..037bc7e1dc 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -7,6 +7,13 @@ #include #include #include +#include "errors.h" +#include "utils.h" + +enum class flag_tag +{ + FT_MUTABLE = 0, /** flag data is mutable */ +}; // Example: // DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read"); @@ -52,6 +59,10 @@ dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ }) +#define DSN_TAG_VARIABLE(name, tag) \ + COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ + static dsn::flag_tagger FLAGS_TAGGER_##name##_##tag(#name, flag_tag::tag) + namespace dsn { // An utility class that registers a flag upon initialization. @@ -74,7 +85,18 @@ class flag_validator flag_validator(const char *name, std::function); }; +class flag_tagger +{ +public: + flag_tagger(const char *name, const flag_tag &tag); +}; + // Loads all the flags from configuration. extern void flags_initialize(); +// update the specified flag to val +extern error_s update_flag(const std::string &name, const std::string &val); + +// determine if the tag is exist for the specified flag +extern bool has_tag(const std::string &name, const flag_tag &tag); } // namespace dsn diff --git a/include/dsn/utility/string_conv.h b/include/dsn/utility/string_conv.h index b89c2ddff7..5d2d8b9fa8 100644 --- a/include/dsn/utility/string_conv.h +++ b/include/dsn/utility/string_conv.h @@ -116,6 +116,11 @@ inline bool buf2int64(string_view buf, int64_t &result) return internal::buf2signed(buf, result); } +inline bool buf2uint32(string_view buf, uint32_t &result) +{ + return internal::buf2unsigned(buf, result); +} + inline bool buf2uint64(string_view buf, uint64_t &result) { return internal::buf2unsigned(buf, result); diff --git a/include/dsn/utility/utils.h b/include/dsn/utility/utils.h index c9e414c93d..fa7085c124 100644 --- a/include/dsn/utility/utils.h +++ b/include/dsn/utility/utils.h @@ -43,6 +43,26 @@ #define TIME_MS_MAX 0xffffffff +// The COMPILE_ASSERT macro can be used to verify that a compile time +// expression is true. For example, you could use it to verify the +// size of a static array: +// +// COMPILE_ASSERT(ARRAYSIZE(content_type_names) == CONTENT_NUM_TYPES, +// content_type_names_incorrect_size); +// +// or to make sure a struct is smaller than a certain size: +// +// COMPILE_ASSERT(sizeof(foo) < 128, foo_too_large); +// +// The second argument to the macro is the name of the variable. If +// the expression is false, most compilers will issue a warning/error +// containing the name of the variable. +struct CompileAssert +{ +}; + +#define COMPILE_ASSERT(expr, msg) static const CompileAssert msg[bool(expr) ? 1 : -1] + namespace dsn { namespace utils { @@ -88,5 +108,5 @@ bool hostname(const dsn::rpc_address &address, std::string *hostname_result); // valid_ip_network_order -> return TRUE && hostname_result=hostname | // invalid_ip_network_order -> return FALSE bool hostname_from_ip(uint32_t ip, std::string *hostname_result); -} -} +} // namespace utils +} // namespace dsn diff --git a/src/http/builtin_http_calls.cpp b/src/http/builtin_http_calls.cpp index 587536b484..b1649ffcf8 100644 --- a/src/http/builtin_http_calls.cpp +++ b/src/http/builtin_http_calls.cpp @@ -72,6 +72,11 @@ namespace dsn { get_perf_counter_handler(req, resp); }) .with_help("Gets the value of a perf counter"); + + register_http_call("updateConfig") + .with_callback( + [](const http_request &req, http_response &resp) { update_config(req, resp); }) + .with_help("Updates the value of a config"); } } // namespace dsn diff --git a/src/http/builtin_http_calls.h b/src/http/builtin_http_calls.h index 2cb190a35f..a218569d19 100644 --- a/src/http/builtin_http_calls.h +++ b/src/http/builtin_http_calls.h @@ -32,4 +32,6 @@ extern void get_help_handler(const http_request &req, http_response &resp); extern void get_recent_start_time_handler(const http_request &req, http_response &resp); +extern void update_config(const http_request &req, http_response &resp); + } // namespace dsn diff --git a/src/http/config_http_service.cpp b/src/http/config_http_service.cpp new file mode 100644 index 0000000000..620b4b7a62 --- /dev/null +++ b/src/http/config_http_service.cpp @@ -0,0 +1,40 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include +#include + +namespace dsn { +void update_config(const http_request &req, http_response &resp) +{ + if (req.query_args.size() != 1) { + resp.status_code = http_status_code::bad_request; + return; + } + + auto iter = req.query_args.begin(); + auto res = update_flag(iter->first, iter->second); + + utils::table_printer tp; + tp.add_row_name_and_data("update_status", res.description()); + std::ostringstream out; + tp.output(out, dsn::utils::table_printer::output_format::kJsonCompact); + resp.body = out.str(); + resp.status_code = http_status_code::ok; +} +} // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 43c6d7b297..bf84e3ff0e 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -5,8 +5,11 @@ #include #include #include +#include +#include #include #include +#include #include @@ -37,6 +40,19 @@ class flag_data } \ break +#define FLAG_DATA_UPDATE_CASE(type, type_enum, suffix) \ + case type_enum: \ + type tmpval_##type_enum; \ + if (!dsn::buf2##suffix(val, tmpval_##type_enum)) { \ + return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} in invalid", val)); \ + } \ + value() = tmpval_##type_enum; \ + break + +#define FLAG_DATA_UPDATE_STRING() \ + case FV_STRING: \ + return error_s::make(ERR_INVALID_PARAMETERS, "string modifications are not supported") + void load() { switch (_type) { @@ -55,9 +71,30 @@ class flag_data { } + error_s update(const std::string &val) + { + if (!has_tag(flag_tag::FT_MUTABLE)) { + return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} is not mutable", _name)); + } + + switch (_type) { + FLAG_DATA_UPDATE_CASE(int32_t, FV_INT32, int32); + FLAG_DATA_UPDATE_CASE(int64_t, FV_INT64, int64); + FLAG_DATA_UPDATE_CASE(uint32_t, FV_UINT32, uint32); + FLAG_DATA_UPDATE_CASE(uint64_t, FV_UINT64, uint64); + FLAG_DATA_UPDATE_CASE(bool, FV_BOOL, bool); + FLAG_DATA_UPDATE_CASE(double, FV_DOUBLE, double); + FLAG_DATA_UPDATE_STRING(); + } + return error_s::make(ERR_OK); + } + void set_validator(validator_fn &validator) { _validator = std::move(validator); } const validator_fn &validator() const { return _validator; } + void add_tag(const flag_tag &tag) { _tags.insert(tag); } + bool has_tag(const flag_tag &tag) const { return _tags.find(tag) != _tags.end(); } + private: template T &value() @@ -72,6 +109,7 @@ class flag_data const char *_name; const char *_desc; validator_fn _validator; + std::unordered_set _tags; }; class flag_registry : public utils::singleton @@ -79,6 +117,15 @@ class flag_registry : public utils::singleton public: void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } + error_s update_flag(const std::string &name, const std::string &val) + { + auto it = _flags.find(name); + if (it == _flags.end()) { + return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("{} is not found", name)); + } + return it->second.update(val); + } + void add_validator(const char *name, validator_fn &validator) { auto it = _flags.find(name); @@ -97,6 +144,22 @@ class flag_registry : public utils::singleton } } + void add_tag(const char *name, const flag_tag &tag) + { + auto it = _flags.find(name); + dassert(it != _flags.end(), "flag \"%s\" does not exist", name); + it->second.add_tag(tag); + } + + bool has_tag(const std::string &name, const flag_tag &tag) const + { + auto it = _flags.find(name); + if (it == _flags.end()) { + return false; + } + return it->second.has_tag(tag); + } + private: friend class utils::singleton; flag_registry() = default; @@ -125,6 +188,21 @@ flag_validator::flag_validator(const char *name, validator_fn validator) flag_registry::instance().add_validator(name, validator); } +flag_tagger::flag_tagger(const char *name, const flag_tag &tag) +{ + flag_registry::instance().add_tag(name, tag); +} + /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } +/*extern*/ error_s update_flag(const std::string &name, const std::string &val) +{ + return flag_registry::instance().update_flag(name, val); +} + +/*extern*/ bool has_tag(const std::string &name, const flag_tag &tag) +{ + return flag_registry::instance().has_tag(name, tag); +} + } // namespace dsn diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp new file mode 100644 index 0000000000..f67c3adaea --- /dev/null +++ b/src/utils/test/flag_test.cpp @@ -0,0 +1,105 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include + +namespace dsn { +namespace utils { + +DSN_DEFINE_int32("flag_test", test_int32, 5, ""); +DSN_TAG_VARIABLE(test_int32, FT_MUTABLE); + +DSN_DEFINE_uint32("flag_test", test_uint32, 5, ""); +DSN_TAG_VARIABLE(test_uint32, FT_MUTABLE); + +DSN_DEFINE_int64("flag_test", test_int64, 5, ""); +DSN_TAG_VARIABLE(test_int64, FT_MUTABLE); + +DSN_DEFINE_uint64("flag_test", test_uint64, 5, ""); +DSN_TAG_VARIABLE(test_uint64, FT_MUTABLE); + +DSN_DEFINE_double("flag_test", test_double, 5.0, ""); +DSN_TAG_VARIABLE(test_double, FT_MUTABLE); + +DSN_DEFINE_bool("flag_test", test_bool, true, ""); +DSN_TAG_VARIABLE(test_bool, FT_MUTABLE); + +DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); + +TEST(flag_test, update_config) +{ + auto res = update_flag("test_int32", "3"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_int32, 3); + + res = update_flag("test_uint32", "3"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_uint32, 3); + + res = update_flag("test_int64", "3"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_int64, 3); + + res = update_flag("test_uint64", "3"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_uint64, 3); + + res = update_flag("test_double", "3.0"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_double, 3.0); + + res = update_flag("test_bool", "false"); + ASSERT_TRUE(res.is_ok()); + ASSERT_FALSE(FLAGS_test_bool); + + // string modifications are not supported + res = update_flag("test_string_immutable", "update_string"); + ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); + ASSERT_EQ(strcmp(FLAGS_test_string_immutable, "immutable_string"), 0); + + // test flag is not exist + res = update_flag("test_not_exist", "test_string"); + ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); + + // test to update invalid value + res = update_flag("test_int32", "3ab"); + ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); + ASSERT_EQ(FLAGS_test_int32, 3); +} + +DSN_DEFINE_int32("flag_test", has_tag, 5, ""); +DSN_TAG_VARIABLE(has_tag, FT_MUTABLE); + +DSN_DEFINE_int32("flag_test", no_tag, 5, ""); + +TEST(flag_test, tag_flag) +{ + // has tag + auto res = has_tag("has_tag", flag_tag::FT_MUTABLE); + ASSERT_TRUE(res); + + // doesn't has tag + res = has_tag("no_tag", flag_tag::FT_MUTABLE); + ASSERT_FALSE(res); + + // flag is not exist + res = has_tag("no_flag", flag_tag::FT_MUTABLE); + ASSERT_FALSE(res); +} +} // namespace utils +} // namespace dsn diff --git a/src/utils/test/main.cpp b/src/utils/test/main.cpp index d64376a3a4..075b224a1c 100644 --- a/src/utils/test/main.cpp +++ b/src/utils/test/main.cpp @@ -1,6 +1,7 @@ #include #include #include +#include extern void command_manager_module_init(); @@ -12,5 +13,7 @@ GTEST_API_ int main(int argc, char **argv) // init logging dsn_log_init("dsn::tools::simple_logger", "./", nullptr); + dsn::flags_initialize(); + return RUN_ALL_TESTS(); } diff --git a/src/utils/test/string_conv_test.cpp b/src/utils/test/string_conv_test.cpp index 5623de1cb9..01607591dd 100644 --- a/src/utils/test/string_conv_test.cpp +++ b/src/utils/test/string_conv_test.cpp @@ -189,6 +189,52 @@ TEST(string_conv, buf2uint64) ASSERT_FALSE(dsn::buf2uint64(dsn::string_view(str.data(), 5), result)); } +TEST(string_conv, buf2uint32) +{ + uint32_t result = 1; + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(0), result)); + ASSERT_EQ(result, 0); + + ASSERT_TRUE(dsn::buf2uint32("-0", result)); + ASSERT_EQ(result, 0); + + ASSERT_FALSE(dsn::buf2uint32("-1", result)); + + ASSERT_TRUE(dsn::buf2uint32("0xdeadbeef", result)); + ASSERT_EQ(result, 0xdeadbeef); + + ASSERT_TRUE(dsn::buf2uint32("0xDEADBEEF", result)); + ASSERT_EQ(result, 0xdeadbeef); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(42), result)); + ASSERT_EQ(result, 42); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::min()), result)); + ASSERT_EQ(result, std::numeric_limits::min()); + + ASSERT_FALSE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + + // "\045" is "%", so the string length=5, otherwise(2th argument > 5) it will be reported + // "global-buffer-overflow" error under AddressSanitizer check + std::string str("123\0456", 5); + ASSERT_TRUE(dsn::buf2uint32(dsn::string_view(str.data(), 2), result)); + ASSERT_EQ(result, 12); + ASSERT_TRUE(dsn::buf2uint32(dsn::string_view(str.data(), 3), result)); + ASSERT_EQ(result, 123); + ASSERT_FALSE(dsn::buf2uint32(dsn::string_view(str.data(), 4), result)); + ASSERT_FALSE(dsn::buf2uint32(dsn::string_view(str.data(), 5), result)); +} + TEST(string_conv, int64_partial) { int64_t result = 0;