From 068be5572c94953447043702d4cc40b0ba8ecde1 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Wed, 29 Jan 2020 10:44:23 +0800 Subject: [PATCH 1/7] utils: use flags to define configuration item --- include/dsn/utility/flags.h | 78 +++++++++++++++ src/core/core/flags.cpp | 127 ++++++++++++++++++++++++ src/core/core/logging.cpp | 37 +++---- src/core/core/service_api_c.cpp | 3 + src/core/tools/common/simple_logger.cpp | 72 ++++++-------- src/core/tools/common/simple_logger.h | 3 - 6 files changed, 252 insertions(+), 68 deletions(-) create mode 100644 include/dsn/utility/flags.h create mode 100644 src/core/core/flags.cpp diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h new file mode 100644 index 0000000000..3faba0fe09 --- /dev/null +++ b/include/dsn/utility/flags.h @@ -0,0 +1,78 @@ +// Copyright (c) 2017-present, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#pragma once + +#include +#include +#include + +// Example: +// DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read") +// DSN_DEFINE_validator(filename, [](const char *fname){ return is_file(fname); }); + +#define DSN_DECLARE_VARIABLE(type, name) extern type FLAGS_##name + +#define DSN_DECLARE_int32(name) DSN_DECLARE_VARIABLE(int32_t, name) +#define DSN_DECLARE_uint32(name) DSN_DECLARE_VARIABLE(uint32_t, name) +#define DSN_DECLARE_int64(name) DSN_DECLARE_VARIABLE(int64_t, name) +#define DSN_DECLARE_uint64(name) DSN_DECLARE_VARIABLE(uint64_t, name) +#define DSN_DECLARE_double(name) DSN_DECLARE_VARIABLE(double, name) +#define DSN_DECLARE_bool(name) DSN_DECLARE_VARIABLE(bool, name) +#define DSN_DECLARE_string(name) DSN_DECLARE_VARIABLE(const char *, name) + +#define DSN_DEFINE_VARIABLE(type, section, name, default_value, desc) \ + type FLAGS_##name = default_value; \ + static dsn::flag_registerer FLAGS_REG_##name(section, #name, desc, &FLAGS_##name) + +#define DSN_DEFINE_int32(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc) +#define DSN_DEFINE_uint32(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc) +#define DSN_DEFINE_int64(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc) +#define DSN_DEFINE_uint64(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc) +#define DSN_DEFINE_double(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(double, section, name, val, desc) +#define DSN_DEFINE_bool(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(bool, section, name, val, desc) +#define DSN_DEFINE_string(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(const char *, section, name, val, desc) + +// Convenience macro for the registration of a flag validator +// `validator` must be a std::function and receives the flag value as argument, +// returns true if validation passed. +#define DSN_DEFINE_validator(name, validator) \ + static auto FLAGS_VALIDATOR_FN_##name = validator; \ + static const dsn::flag_validator FLAGS_VALIDATOR_##name(#name, []() { \ + dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ + }) + +namespace dsn { + +// An utility class that registers a flag upon initialization. +class flag_registerer +{ +public: + flag_registerer(const char *section, const char *name, const char *desc, int32_t *val); + flag_registerer(const char *section, const char *name, const char *desc, uint32_t *val); + flag_registerer(const char *section, const char *name, const char *desc, int64_t *val); + flag_registerer(const char *section, const char *name, const char *desc, uint64_t *val); + flag_registerer(const char *section, const char *name, const char *desc, double *val); + flag_registerer(const char *section, const char *name, const char *desc, bool *val); + flag_registerer(const char *section, const char *name, const char *desc, const char **val); +}; + +// An utility class that registers a validator upon initialization. +class flag_validator +{ +public: + flag_validator(const char *name, std::function); +}; + +// Loads all the flags from configuration. +extern void flags_initialize(); + +} // namespace dsn diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp new file mode 100644 index 0000000000..7e4448c9ca --- /dev/null +++ b/src/core/core/flags.cpp @@ -0,0 +1,127 @@ +// Copyright (c) 2017-present, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#include +#include +#include +#include +#include + +#include + +namespace dsn { + +enum value_type +{ + FV_BOOL = 0, + FV_INT32 = 1, + FV_UINT32 = 2, + FV_INT64 = 3, + FV_UINT64 = 4, + FV_DOUBLE = 5, + FV_STRING = 6, + FV_MAX_INDEX = 6, +}; + +using validator_fn = std::function; + +class flag_data +{ +public: +#define FLAG_DATA_LOAD_CASE(type, type_enum, suffix) \ + case type_enum: \ + value() = dsn_config_get_value_##suffix(_section, _name, value(), _desc); \ + if (_validator) { \ + _validator(); \ + } \ + break + + void load() + { + switch (_type) { + FLAG_DATA_LOAD_CASE(int32_t, FV_INT32, int64); + FLAG_DATA_LOAD_CASE(int64_t, FV_INT64, int64); + FLAG_DATA_LOAD_CASE(uint32_t, FV_UINT32, uint64); + FLAG_DATA_LOAD_CASE(uint64_t, FV_UINT64, uint64); + FLAG_DATA_LOAD_CASE(bool, FV_BOOL, bool); + FLAG_DATA_LOAD_CASE(double, FV_DOUBLE, double); + FLAG_DATA_LOAD_CASE(const char *, FV_STRING, string); + } + } + + flag_data(const char *section, const char *name, const char *desc, value_type type, void *val) + : _type(type), _val(val), _section(section), _name(name), _desc(desc) + { + } + + void set_validator(validator_fn &validator) { _validator = std::move(validator); } + const validator_fn &validator() const { return _validator; } + +private: + template + T &value() + { + return *reinterpret_cast(_val); + } + +private: + const value_type _type; + void *const _val; + const char *_section; + const char *_name; + const char *_desc; + validator_fn _validator; +}; + +class flag_registry : public utils::singleton +{ +public: + flag_registry() {} + + void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } + + void add_validator(const char *name, validator_fn &validator) + { + auto it = _flags.find(name); + dassert(it != _flags.end(), "flag \"%s\" does not exist", name); + flag_data &flag = it->second; + dassert(!flag.validator(), "\"%s\" validator already registered", name); + flag.set_validator(validator); + } + + void load_from_config() + { + for (auto &kv : _flags) { + flag_data &flag = kv.second; + flag.load(); + } + } + +private: + std::map _flags; +}; + +#define FLAG_REG_CONSTRUCTOR(type, type_enum) \ + flag_registerer::flag_registerer( \ + const char *section, const char *name, const char *desc, type *val) \ + { \ + flag_registry::instance().add_flag(name, flag_data(section, name, desc, type_enum, val)); \ + } + +FLAG_REG_CONSTRUCTOR(int32_t, FV_INT32); +FLAG_REG_CONSTRUCTOR(uint32_t, FV_UINT32); +FLAG_REG_CONSTRUCTOR(int64_t, FV_INT64); +FLAG_REG_CONSTRUCTOR(uint64_t, FV_UINT64); +FLAG_REG_CONSTRUCTOR(bool, FV_BOOL); +FLAG_REG_CONSTRUCTOR(double, FV_DOUBLE); +FLAG_REG_CONSTRUCTOR(const char *, FV_STRING); + +flag_validator::flag_validator(const char *name, validator_fn validator) +{ + flag_registry::instance().add_validator(name, validator); +} + +/*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } + +} // namespace dsn diff --git a/src/core/core/logging.cpp b/src/core/core/logging.cpp index a98cd686d0..619e8fc417 100644 --- a/src/core/core/logging.cpp +++ b/src/core/core/logging.cpp @@ -24,24 +24,21 @@ * THE SOFTWARE. */ -/* - * Description: - * Description: - * What is this file about? - * - * Revision history: - * xxxx-xx-xx, author, first version - * xxxx-xx-xx, author, fix bug about xxx - */ - #include #include #include #include #include "service_engine.h" #include +#include DSN_API dsn_log_level_t dsn_log_start_level = dsn_log_level_t::LOG_LEVEL_INFORMATION; +DSN_DEFINE_string("core", + logging_start_level, + "LOG_LEVEL_INFORMATION", + "logs with level below this will not be logged"); + +DSN_DEFINE_bool("core", logging_flush_on_exit, true, "flush log when exit system"); static void log_on_sys_exit(::dsn::sys_exit_type) { @@ -53,20 +50,14 @@ static void log_on_sys_exit(::dsn::sys_exit_type) void dsn_log_init() { - dsn_log_start_level = enum_from_string( - dsn_config_get_value_string("core", - "logging_start_level", - enum_to_string(dsn_log_start_level), - "logs with level below this will not be logged"), - dsn_log_level_t::LOG_LEVEL_INVALID); + dsn_log_start_level = + enum_from_string(FLAGS_logging_start_level, dsn_log_level_t::LOG_LEVEL_INVALID); dassert(dsn_log_start_level != dsn_log_level_t::LOG_LEVEL_INVALID, "invalid [core] logging_start_level specified"); // register log flush on exit - bool logging_flush_on_exit = dsn_config_get_value_bool( - "core", "logging_flush_on_exit", true, "flush log when exit system"); - if (logging_flush_on_exit) { + if (FLAGS_logging_flush_on_exit) { ::dsn::tools::sys_exit.put_back(log_on_sys_exit, "log.flush"); } @@ -89,12 +80,8 @@ void dsn_log_init() [](const std::vector &args) { dsn_log_level_t start_level; if (args.size() == 0) { - start_level = enum_from_string( - dsn_config_get_value_string("core", - "logging_start_level", - enum_to_string(dsn_log_start_level), - "logs with level below this will not be logged"), - dsn_log_level_t::LOG_LEVEL_INVALID); + start_level = + enum_from_string(FLAGS_logging_start_level, dsn_log_level_t::LOG_LEVEL_INVALID); } else { std::string level_str = "LOG_LEVEL_" + args[0]; start_level = diff --git a/src/core/core/service_api_c.cpp b/src/core/core/service_api_c.cpp index bb1a57ecb6..404064e5c3 100644 --- a/src/core/core/service_api_c.cpp +++ b/src/core/core/service_api_c.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #ifdef DSN_ENABLE_GPERF @@ -318,6 +319,8 @@ bool run(const char *config_file, return false; } + dsn::flags_initialize(); + // pause when necessary if (dsn_config_get_value_bool("core", "pause_on_start", diff --git a/src/core/tools/common/simple_logger.cpp b/src/core/tools/common/simple_logger.cpp index c89e73cb85..29cbef01ee 100644 --- a/src/core/tools/common/simple_logger.cpp +++ b/src/core/tools/common/simple_logger.cpp @@ -24,22 +24,34 @@ * THE SOFTWARE. */ -/* - * Description: - * What is this file about? - * - * Revision history: - * xxxx-xx-xx, author, first version - * xxxx-xx-xx, author, fix bug about xxx - */ - #include "simple_logger.h" #include #include +#include namespace dsn { namespace tools { +DSN_DEFINE_bool("tools.simple_logger", fast_flush, false, "whether to flush immediately"); + +DSN_DEFINE_bool("tools.simple_logger", + short_header, + true, + "whether to use short header (excluding file/function etc.)"); + +DSN_DEFINE_uint64("tools.simple_logger", + max_number_of_log_files_on_disk, + 20, + "max number of log files reserved on disk, older logs are auto deleted"); + +DSN_DEFINE_string("tools.simple_logger", + stderr_start_level, + "LOG_LEVEL_WARNING", + "copy log messages at or above this level to stderr in addition to logfiles"); +DSN_DEFINE_validator(stderr_start_level, [](const char *level) -> bool { + return strcmp(level, "LOG_LEVEL_INVALID") != 0; +}); + static void print_header(FILE *fp, dsn_log_level_t log_level) { static char s_level_char[] = "IDWEF"; @@ -123,28 +135,7 @@ simple_logger::simple_logger(const char *log_dir) : logging_provider(log_dir) _index = 1; _lines = 0; _log = nullptr; - _short_header = - dsn_config_get_value_bool("tools.simple_logger", - "short_header", - true, - "whether to use short header (excluding file/function etc.)"); - _fast_flush = dsn_config_get_value_bool( - "tools.simple_logger", "fast_flush", false, "whether to flush immediately"); - _stderr_start_level = enum_from_string( - dsn_config_get_value_string( - "tools.simple_logger", - "stderr_start_level", - enum_to_string(LOG_LEVEL_WARNING), - "copy log messages at or above this level to stderr in addition to logfiles"), - LOG_LEVEL_INVALID); - dassert(_stderr_start_level != LOG_LEVEL_INVALID, - "invalid [tools.simple_logger] stderr_start_level specified"); - - _max_number_of_log_files_on_disk = dsn_config_get_value_uint64( - "tools.simple_logger", - "max_number_of_log_files_on_disk", - 20, - "max number of log files reserved on disk, older logs are auto deleted"); + _stderr_start_level = enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID); // check existing log files std::vector sub_list; @@ -188,7 +179,7 @@ void simple_logger::create_log_file() _log = ::fopen(str.str().c_str(), "w+"); // TODO: move gc out of criticial path - while (_index - _start_index > _max_number_of_log_files_on_disk) { + while (_index - _start_index > FLAGS_max_number_of_log_files_on_disk) { std::stringstream str2; str2 << "log." << _start_index++ << ".txt"; auto dp = utils::filesystem::path_combine(_log_dir, str2.str()); @@ -229,18 +220,18 @@ void simple_logger::dsn_logv(const char *file, utils::auto_lock<::dsn::utils::ex_lock> l(_lock); print_header(_log, log_level); - if (!_short_header) { + if (!FLAGS_short_header) { fprintf(_log, "%s:%d:%s(): ", file, line, function); } vfprintf(_log, fmt, args); fprintf(_log, "\n"); - if (_fast_flush || log_level >= LOG_LEVEL_ERROR) { + if (FLAGS_fast_flush || log_level >= LOG_LEVEL_ERROR) { ::fflush(_log); } if (log_level >= _stderr_start_level) { print_header(stdout, log_level); - if (!_short_header) { + if (!FLAGS_short_header) { printf("%s:%d:%s(): ", file, line, function); } vprintf(fmt, args2); @@ -261,17 +252,17 @@ void simple_logger::dsn_log(const char *file, utils::auto_lock<::dsn::utils::ex_lock> l(_lock); print_header(_log, log_level); - if (!_short_header) { + if (!FLAGS_short_header) { fprintf(_log, "%s:%d:%s(): ", file, line, function); } fprintf(_log, "%s\n", str); - if (_fast_flush || log_level >= LOG_LEVEL_ERROR) { + if (FLAGS_fast_flush || log_level >= LOG_LEVEL_ERROR) { ::fflush(_log); } if (log_level >= _stderr_start_level) { print_header(stdout, log_level); - if (!_short_header) { + if (!FLAGS_short_header) { printf("%s:%d:%s(): ", file, line, function); } printf("%s\n", str); @@ -281,5 +272,6 @@ void simple_logger::dsn_log(const char *file, create_log_file(); } } -} -} + +} // namespace tools +} // namespace dsn diff --git a/src/core/tools/common/simple_logger.h b/src/core/tools/common/simple_logger.h index c58814727d..f29bd9b53c 100644 --- a/src/core/tools/common/simple_logger.h +++ b/src/core/tools/common/simple_logger.h @@ -100,10 +100,7 @@ class simple_logger : public logging_provider int _start_index; int _index; int _lines; - bool _short_header; - bool _fast_flush; dsn_log_level_t _stderr_start_level; - int _max_number_of_log_files_on_disk; }; } } From 5c880ef4fc8f3d36b568b3bf698f6c32cd6f9fcf Mon Sep 17 00:00:00 2001 From: neverchanje Date: Thu, 30 Jan 2020 20:46:56 +0800 Subject: [PATCH 2/7] fix --- include/dsn/utility/flags.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index 3faba0fe09..c312494dfa 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -9,8 +9,9 @@ #include // Example: -// DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read") +// DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read"); // DSN_DEFINE_validator(filename, [](const char *fname){ return is_file(fname); }); +// auto fptr = file::open(FLAGS_filename, O_RDONLY | O_BINARY, 0); #define DSN_DECLARE_VARIABLE(type, name) extern type FLAGS_##name @@ -41,9 +42,10 @@ #define DSN_DEFINE_string(section, name, val, desc) \ DSN_DEFINE_VARIABLE(const char *, section, name, val, desc) -// Convenience macro for the registration of a flag validator +// Convenience macro for the registration of a flag validator. // `validator` must be a std::function and receives the flag value as argument, // returns true if validation passed. +// The program corrupts if the validation failed. #define DSN_DEFINE_validator(name, validator) \ static auto FLAGS_VALIDATOR_FN_##name = validator; \ static const dsn::flag_validator FLAGS_VALIDATOR_##name(#name, []() { \ From 8570fbc158f07a93f5c1270774a82dde177fa81e Mon Sep 17 00:00:00 2001 From: neverchanje Date: Thu, 30 Jan 2020 21:34:53 +0800 Subject: [PATCH 3/7] fix ut --- src/core/core/flags.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp index 7e4448c9ca..5fffbb8bd7 100644 --- a/src/core/core/flags.cpp +++ b/src/core/core/flags.cpp @@ -86,8 +86,9 @@ class flag_registry : public utils::singleton auto it = _flags.find(name); dassert(it != _flags.end(), "flag \"%s\" does not exist", name); flag_data &flag = it->second; - dassert(!flag.validator(), "\"%s\" validator already registered", name); - flag.set_validator(validator); + if (!flag.validator()) { + flag.set_validator(validator); + } } void load_from_config() From ab6715853c3f89127e27a766e5ac75feefde7c35 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Mon, 27 Apr 2020 23:45:49 +0800 Subject: [PATCH 4/7] Update flags.cpp --- src/core/core/flags.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp index 5fffbb8bd7..b5737507b1 100644 --- a/src/core/core/flags.cpp +++ b/src/core/core/flags.cpp @@ -77,8 +77,6 @@ class flag_data class flag_registry : public utils::singleton { public: - flag_registry() {} - void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } void add_validator(const char *name, validator_fn &validator) @@ -98,6 +96,9 @@ class flag_registry : public utils::singleton flag.load(); } } + +private: + flag_registry() = default; private: std::map _flags; From c330e6633a08bfe4e79bd9c724190548d7df036f Mon Sep 17 00:00:00 2001 From: neverchanje Date: Tue, 28 Apr 2020 09:44:28 +0800 Subject: [PATCH 5/7] fix --- src/core/core/flags.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp index b5737507b1..7f3c5a5867 100644 --- a/src/core/core/flags.cpp +++ b/src/core/core/flags.cpp @@ -96,7 +96,7 @@ class flag_registry : public utils::singleton flag.load(); } } - + private: flag_registry() = default; From ffc736e26e7c99c2391036923ce55dcedd6cf29b Mon Sep 17 00:00:00 2001 From: neverchanje Date: Tue, 28 Apr 2020 09:56:31 +0800 Subject: [PATCH 6/7] fix --- src/core/core/flags.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp index 7f3c5a5867..0cc326025b 100644 --- a/src/core/core/flags.cpp +++ b/src/core/core/flags.cpp @@ -77,6 +77,8 @@ class flag_data class flag_registry : public utils::singleton { public: + flag_registry() = default; + void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } void add_validator(const char *name, validator_fn &validator) @@ -97,9 +99,6 @@ class flag_registry : public utils::singleton } } -private: - flag_registry() = default; - private: std::map _flags; }; From 7ded91620aa5fed39c1cee125ad08fef739f89ac Mon Sep 17 00:00:00 2001 From: neverchanje Date: Wed, 29 Apr 2020 14:58:17 +0800 Subject: [PATCH 7/7] fix --- src/core/core/flags.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/core/flags.cpp b/src/core/core/flags.cpp index 0cc326025b..43c6d7b297 100644 --- a/src/core/core/flags.cpp +++ b/src/core/core/flags.cpp @@ -77,8 +77,6 @@ class flag_data class flag_registry : public utils::singleton { public: - flag_registry() = default; - void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } void add_validator(const char *name, validator_fn &validator) @@ -99,6 +97,10 @@ class flag_registry : public utils::singleton } } +private: + friend class utils::singleton; + flag_registry() = default; + private: std::map _flags; };