From aa5da119aa6b24b8a8ebc6c35fcc785d3008765d Mon Sep 17 00:00:00 2001 From: fuzhe1989 Date: Thu, 21 Jul 2022 12:40:58 +0800 Subject: [PATCH 1/6] update Signed-off-by: fuzhe1989 --- dbms/src/Common/Exception.cpp | 52 +++-- dbms/src/Common/Exception.h | 179 ++++++++++++++---- dbms/src/Common/Logger.h | 64 ++++--- .../Coprocessor/DAGQueryBlockInterpreter.cpp | 4 +- dbms/src/Flash/Mpp/ExchangeReceiver.cpp | 2 +- dbms/src/TestUtils/FunctionTestUtils.h | 4 +- 6 files changed, 216 insertions(+), 89 deletions(-) diff --git a/dbms/src/Common/Exception.cpp b/dbms/src/Common/Exception.cpp index 5e95156eb5d..e04d8a642c8 100644 --- a/dbms/src/Common/Exception.cpp +++ b/dbms/src/Common/Exception.cpp @@ -24,7 +24,6 @@ #include #include - namespace DB { namespace ErrorCodes @@ -35,7 +34,6 @@ extern const int UNKNOWN_EXCEPTION; extern const int CANNOT_TRUNCATE_FILE; } // namespace ErrorCodes - void throwFromErrno(const std::string & s, int code, int e) { const size_t buf_size = 128; @@ -54,14 +52,18 @@ void throwFromErrno(const std::string & s, int code, int e) strcpy(buf, unknown_message); strcpy(buf + strlen(unknown_message), code); } - throw ErrnoException(s + ", errno: " + toString(e) + ", strerror: " + std::string(buf), code, e); + throw ErrnoException(s + ", errno: " + toString(e) + ", strerror: " + std::string(buf), + code, + e); #else - throw ErrnoException(s + ", errno: " + toString(e) + ", strerror: " + std::string(strerror_r(e, buf, sizeof(buf))), code, e); + throw ErrnoException(s + ", errno: " + toString(e) + ", strerror: " + std::string(strerror_r(e, buf, sizeof(buf))), + code, + e); #endif } - -void tryLogCurrentException(const char * log_name, const std::string & start_of_message) +void tryLogCurrentException(const char * log_name, + const std::string & start_of_message) { tryLogCurrentException(&Poco::Logger::get(log_name), start_of_message); } @@ -75,19 +77,22 @@ void tryLogCurrentException(const char * log_name, const std::string & start_of_ { \ } -void tryLogCurrentException(const LoggerPtr & logger, const std::string & start_of_message) +void tryLogCurrentException(const LoggerPtr & logger, + const std::string & start_of_message) { TRY_LOG_CURRENT_EXCEPTION(logger, start_of_message); } -void tryLogCurrentException(Poco::Logger * logger, const std::string & start_of_message) +void tryLogCurrentException(Poco::Logger * logger, + const std::string & start_of_message) { TRY_LOG_CURRENT_EXCEPTION(logger, start_of_message); } #undef TRY_LOG_CURRENT_EXCEPTION -std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded_stacktrace) +std::string getCurrentExceptionMessage(bool with_stacktrace, + bool check_embedded_stacktrace) { std::stringstream stream; @@ -103,8 +108,10 @@ std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded { try { - stream << "Poco::Exception. Code: " << ErrorCodes::POCO_EXCEPTION << ", e.code() = " << e.code() - << ", e.displayText() = " << e.displayText() << ", e.what() = " << e.what(); + stream << "Poco::Exception. Code: " << ErrorCodes::POCO_EXCEPTION + << ", e.code() = " << e.code() + << ", e.displayText() = " << e.displayText() + << ", e.what() = " << e.what(); } catch (...) { @@ -120,7 +127,8 @@ std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded if (status) name += " (demangling status: " + toString(status) + ")"; - stream << "std::exception. Code: " << ErrorCodes::STD_EXCEPTION << ", type: " << name << ", e.what() = " << e.what(); + stream << "std::exception. Code: " << ErrorCodes::STD_EXCEPTION + << ", type: " << name << ", e.what() = " << e.what(); } catch (...) { @@ -136,7 +144,8 @@ std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded if (status) name += " (demangling status: " + toString(status) + ")"; - stream << "Unknown exception. Code: " << ErrorCodes::UNKNOWN_EXCEPTION << ", type: " << name; + stream << "Unknown exception. Code: " << ErrorCodes::UNKNOWN_EXCEPTION + << ", type: " << name; } catch (...) { @@ -146,7 +155,6 @@ std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded return stream.str(); } - int getCurrentExceptionCode() { try @@ -171,7 +179,6 @@ int getCurrentExceptionCode() } } - void rethrowFirstException(const Exceptions & exceptions) { for (const auto & exception : exceptions) @@ -179,7 +186,6 @@ void rethrowFirstException(const Exceptions & exceptions) std::rethrow_exception(exception); } - std::string getExceptionMessage(const Exception & e, bool with_stacktrace, bool check_embedded_stacktrace) { std::stringstream stream; @@ -200,7 +206,8 @@ std::string getExceptionMessage(const Exception & e, bool with_stacktrace, bool } } - stream << "Code: " << e.code() << ", e.displayText() = " << text << ", e.what() = " << e.what(); + stream << "Code: " << e.code() << ", e.displayText() = " << text + << ", e.what() = " << e.what(); if (with_stacktrace && !has_embedded_stack_trace) stream << ", Stack trace:\n\n" @@ -225,7 +232,6 @@ std::string getExceptionMessage(std::exception_ptr e, bool with_stacktrace) } } - std::string ExecutionStatus::serializeText() const { WriteBufferFromOwnString wb; @@ -254,11 +260,19 @@ bool ExecutionStatus::tryDeserializeText(const std::string & data) return true; } -ExecutionStatus ExecutionStatus::fromCurrentException(const std::string & start_of_message) +ExecutionStatus +ExecutionStatus::fromCurrentException(const std::string & start_of_message) { String msg = (start_of_message.empty() ? "" : (start_of_message + ": ")) + getCurrentExceptionMessage(false, true); return ExecutionStatus(getCurrentExceptionCode(), msg); } +namespace exception_details +{ +Poco::Logger * getDefaultFatalLogger() +{ + return &Poco::Logger::get("DefaultFatal"); +} +} // namespace exception_details } // namespace DB diff --git a/dbms/src/Common/Exception.h b/dbms/src/Common/Exception.h index 3322c99bce0..2a6fd7bbaef 100644 --- a/dbms/src/Common/Exception.h +++ b/dbms/src/Common/Exception.h @@ -14,26 +14,17 @@ #pragma once +#include #include #include -#include +#include #include #include #include - -namespace Poco -{ -class Logger; -} - - namespace DB { -class Logger; -using LoggerPtr = std::shared_ptr; - class Exception : public Poco::Exception { public: @@ -73,8 +64,8 @@ class Exception : public Poco::Exception StackTrace trace; }; - -/// Contains an additional member `saved_errno`. See the throwFromErrno function. +/// Contains an additional member `saved_errno`. See the throwFromErrno +/// function. class ErrnoException : public Exception { public: @@ -97,33 +88,33 @@ class ErrnoException : public Exception int saved_errno; }; - using Exceptions = std::vector; - [[noreturn]] void throwFromErrno(const std::string & s, int code = 0, int e = errno); - /** Try to write an exception to the log (and forget about it). - * Can be used in destructors in the catch-all block. - */ -void tryLogCurrentException(const char * log_name, const std::string & start_of_message = ""); -void tryLogCurrentException(const LoggerPtr & logger, const std::string & start_of_message = ""); -void tryLogCurrentException(Poco::Logger * logger, const std::string & start_of_message = ""); - + * Can be used in destructors in the catch-all block. + */ +void tryLogCurrentException(const char * log_name, + const std::string & start_of_message = ""); +void tryLogCurrentException(const LoggerPtr & logger, + const std::string & start_of_message = ""); +void tryLogCurrentException(Poco::Logger * logger, + const std::string & start_of_message = ""); /** Prints current exception in canonical format. - * with_stacktrace - prints stack trace for DB::Exception. - * check_embedded_stacktrace - if DB::Exception has embedded stacktrace then - * only this stack trace will be printed. - */ -std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded_stacktrace = false); + * with_stacktrace - prints stack trace for DB::Exception. + * check_embedded_stacktrace - if DB::Exception has embedded stacktrace then + * only this stack trace will be printed. + */ +std::string getCurrentExceptionMessage(bool with_stacktrace, + bool check_embedded_stacktrace = false); /// Returns error code from ErrorCodes int getCurrentExceptionCode(); - -/// An execution status of any piece of code, contains return code and optional error +/// An execution status of any piece of code, contains return code and optional +/// error struct ExecutionStatus { int code = 0; @@ -131,12 +122,14 @@ struct ExecutionStatus ExecutionStatus() = default; - explicit ExecutionStatus(int return_code, const std::string & exception_message = "") + explicit ExecutionStatus(int return_code, + const std::string & exception_message = "") : code(return_code) , message(exception_message) {} - static ExecutionStatus fromCurrentException(const std::string & start_of_message = ""); + static ExecutionStatus + fromCurrentException(const std::string & start_of_message = ""); std::string serializeText() const; @@ -145,14 +138,11 @@ struct ExecutionStatus bool tryDeserializeText(const std::string & data); }; - std::string getExceptionMessage(const Exception & e, bool with_stacktrace, bool check_embedded_stacktrace = false); std::string getExceptionMessage(std::exception_ptr e, bool with_stacktrace); - void rethrowFirstException(const Exceptions & exceptions); - template std::enable_if_t, T> exception_cast(std::exception_ptr e) { @@ -172,21 +162,104 @@ std::enable_if_t, T> exception_cast(std::exception_ptr e) namespace exception_details { +inline std::string generateFormattedMessage(const char * condition) +{ + return fmt::format("Assert {} fail!", condition); +} + template -inline std::string generateLogMessage(const char * condition, T && fmt_str, Args &&... args) +inline std::string generateFormattedMessage(const char * condition, T && fmt_str, Args &&... args) +{ + return FmtBuffer().fmtAppend("Assert {} fail! ", condition).fmtAppend(fmt_str, std::forward(args)...).toString(); +} + +template +inline Poco::Message generateLogMessage(const std::string & logger_name, const char * filename, int lineno, const char * condition, Args &&... args) +{ + return Poco::Message( + logger_name, + generateFormattedMessage(condition, std::forward(args)...), + Poco::Message::PRIO_FATAL, + filename, + lineno); +} + +Poco::Logger * getDefaultFatalLogger(); + +inline void log(const char * filename, int lineno, const char * condition, Poco::Logger * logger) { - return fmt::format(std::forward(fmt_str), condition, std::forward(args)...); + if (likely(logger->fatal())) + { + auto message = generateLogMessage(logger->name(), filename, lineno, condition); + logger->log(message); + } +} + +inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger) +{ + if (likely(logger->fatal())) + { + auto message = generateLogMessage(logger->name(), filename, lineno, condition); + logger->log(message); + } +} + +inline void log(const char * filename, int lineno, const char * condition) +{ + log(filename, lineno, condition, getDefaultFatalLogger()); +} + +template +inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger, const char * fmt_str, Args &&... args) +{ + if (logger->fatal()) + { + auto message = generateLogMessage( + logger->name(), + filename, + lineno, + condition, + fmt_str, + std::forward(args)...); + logger->log(message); + } +} + +template +inline void log(const char * filename, int lineno, const char * condition, Poco::Logger * logger, const char * fmt_str, Args &&... args) +{ + if (logger->fatal()) + { + auto message = generateLogMessage( + logger->name(), + filename, + lineno, + condition, + fmt_str, + std::forward(args)...); + logger->log(message); + } +} + +template +inline void log(const char * filename, int lineno, const char * condition, const char * fmt_str, Args &&... args) +{ + log(filename, lineno, condition, getDefaultFatalLogger(), fmt_str, std::forward(args)...); } } // namespace exception_details -#define RUNTIME_CHECK(condition, ExceptionType, ...) \ - do \ - { \ - if (unlikely(!(condition))) \ - throw ExceptionType(__VA_ARGS__); \ +/// Usage: +/// ``` +/// RUNTIME_CHECK(a != b, Exception("{} does not equal to {}", a, b)); +/// ``` +#define RUNTIME_CHECK(condition, ExceptionGenerationCode) \ + do \ + { \ + if (unlikely(!(condition))) \ + throw(ExceptionGenerationCode); \ } while (false) -#define RUNTIME_ASSERT(condition, logger, ...) \ +#define RUNTIME_ASSERT_IMPL(condition, logger, ...) \ do \ { \ if (unlikely(!(condition))) \ @@ -196,4 +269,26 @@ inline std::string generateLogMessage(const char * condition, T && fmt_str, Args } \ } while (false) +/// Usage: +/// ``` +/// RUNTIME_ASSERT(a != b); +/// RUNTIME_ASSERT(a != b, "fail"); +/// RUNTIME_ASSERT(a != b, "{} does not equal to {}", a, b); +/// RUNTIME_ASSERT(a != b, logger); +/// RUNTIME_ASSERT(a != b, logger, "{} does not equal to {}", a, b); +/// ``` +#define RUNTIME_ASSERT(condition, ...) \ + do \ + { \ + if (unlikely(!(condition))) \ + { \ + exception_details::log( \ + &__FILE__[LogFmtDetails::getFileNameOffset(__FILE__)], \ + __LINE__, \ + #condition, \ + __VA_ARGS__); \ + std::terminate(); \ + } \ + } while (false) + } // namespace DB diff --git a/dbms/src/Common/Logger.h b/dbms/src/Common/Logger.h index 02aaa4d8cbe..d457dfac294 100644 --- a/dbms/src/Common/Logger.h +++ b/dbms/src/Common/Logger.h @@ -27,13 +27,15 @@ using LoggerPtr = std::shared_ptr; /** * Logger is to support identifiers based on Poco::Logger. * - * Identifiers could be request_id, session_id, etc. They can be used in `LogSearch` when we want to - * glob all logs related to one request/session/query. + * Identifiers could be request_id, session_id, etc. They can be used in + * `LogSearch` when we want to glob all logs related to one + * request/session/query. * - * Logger will print all identifiers at the front of each log record (and after the `source`). + * Logger will print all identifiers at the front of each log record (and after + * the `source`). * - * Interfaces in Logger are definitely the same with the Poco::Logger, so that they could use the same - * macro such as LOG_INFO() etc. + * Interfaces in Logger are definitely the same with the Poco::Logger, so that + * they could use the same macro such as LOG_INFO() etc. */ class Logger : private boost::noncopyable { @@ -59,23 +61,24 @@ class Logger : private boost::noncopyable Logger(const std::string & source, const std::string & identifier) : Logger(&Poco::Logger::get(source), identifier) - { - } + {} Logger(Poco::Logger * source_log, const std::string & identifier) : logger(source_log) , id(identifier) - { - } - -#define M(level) \ - bool level() const { return logger->level(); } \ - void level(const std::string & msg) const \ - { \ - if (id.empty()) \ - logger->level(msg); \ - else \ - logger->level(wrapMsg(msg)); \ + {} + +#define M(level) \ + bool level() const \ + { \ + return logger->level(); \ + } \ + void level(const std::string & msg) const \ + { \ + if (id.empty()) \ + logger->level(msg); \ + else \ + logger->level(wrapMsg(msg)); \ } M(trace) @@ -101,15 +104,30 @@ class Logger : private boost::noncopyable return logger->log(msg); } - bool is(int level) const { return logger->is(level); } + bool is(int level) const + { + return logger->is(level); + } - Poco::Channel * getChannel() const { return logger->getChannel(); } + Poco::Channel * getChannel() const + { + return logger->getChannel(); + } - const std::string & name() const { return logger->name(); } + const std::string & name() const + { + return logger->name(); + } - const std::string & identifier() const { return id; } + const std::string & identifier() const + { + return id; + } - Poco::Logger * getLog() const { return logger; } + Poco::Logger * getLog() const + { + return logger; + } private: template diff --git a/dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp b/dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp index 764bf07f533..586e434f84a 100644 --- a/dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp +++ b/dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp @@ -743,8 +743,8 @@ void DAGQueryBlockInterpreter::handleExchangeSender(DAGPipeline & pipeline) stream = std::make_shared(stream, std::move(response_writer), log->identifier()); stream->setExtraInfo(enableFineGrainedShuffleExtraInfo); }); - RUNTIME_CHECK(exchange_sender.tp() == tipb::ExchangeType::Hash, Exception, "exchange_sender has to be hash partition when fine grained shuffle is enabled"); - RUNTIME_CHECK(stream_count <= 1024, Exception, "fine_grained_shuffle_stream_count should not be greater than 1024"); + RUNTIME_CHECK(exchange_sender.tp() == tipb::ExchangeType::Hash, Exception("exchange_sender has to be hash partition when fine grained shuffle is enabled")); + RUNTIME_CHECK(stream_count <= 1024, Exception("fine_grained_shuffle_stream_count should not be greater than 1024")); } else { diff --git a/dbms/src/Flash/Mpp/ExchangeReceiver.cpp b/dbms/src/Flash/Mpp/ExchangeReceiver.cpp index ab8d83a1481..f5808952740 100644 --- a/dbms/src/Flash/Mpp/ExchangeReceiver.cpp +++ b/dbms/src/Flash/Mpp/ExchangeReceiver.cpp @@ -750,7 +750,7 @@ ExchangeReceiverResult ExchangeReceiverBase::nextResult(std::queuechunks.empty()); // Fine grained shuffle should only be enabled when sending data to TiFlash node. // So all data should be encoded into MPPDataPacket.chunks. - RUNTIME_CHECK(!enableFineGrainedShuffle(fine_grained_shuffle_stream_count), Exception, "Data should not be encoded into tipb::SelectResponse.chunks when fine grained shuffle is enabled"); + RUNTIME_CHECK(!enableFineGrainedShuffle(fine_grained_shuffle_stream_count), Exception("Data should not be encoded into tipb::SelectResponse.chunks when fine grained shuffle is enabled")); result.decode_detail = CoprocessorReader::decodeChunks(select_resp, block_queue, header, schema); } } diff --git a/dbms/src/TestUtils/FunctionTestUtils.h b/dbms/src/TestUtils/FunctionTestUtils.h index 8680d1886b1..e75acabe56c 100644 --- a/dbms/src/TestUtils/FunctionTestUtils.h +++ b/dbms/src/TestUtils/FunctionTestUtils.h @@ -398,7 +398,7 @@ typename TypeTraits::FieldType parseDecimal( const String & literal = [&] { if constexpr (Traits::is_nullable) { - assert(literal_.has_value()); + RUNTIME_ASSERT(literal_.has_value()); return literal_.value(); } else @@ -761,7 +761,7 @@ class FunctionTest : public ::testing::Test DAGContext & getDAGContext() { - assert(dag_context_ptr != nullptr); + RUNTIME_ASSERT(dag_context_ptr != nullptr); return *dag_context_ptr; } From 791f3437365d869507d23243a9d51c4241a10a03 Mon Sep 17 00:00:00 2001 From: Fu Zhe Date: Fri, 22 Jul 2022 14:09:34 +0800 Subject: [PATCH 2/6] Update Exception.h --- dbms/src/Common/Exception.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/dbms/src/Common/Exception.h b/dbms/src/Common/Exception.h index 2a6fd7bbaef..fd3b30b9c92 100644 --- a/dbms/src/Common/Exception.h +++ b/dbms/src/Common/Exception.h @@ -259,16 +259,6 @@ inline void log(const char * filename, int lineno, const char * condition, const throw(ExceptionGenerationCode); \ } while (false) -#define RUNTIME_ASSERT_IMPL(condition, logger, ...) \ - do \ - { \ - if (unlikely(!(condition))) \ - { \ - LOG_FATAL((logger), exception_details::generateLogMessage(#condition, "Assert {} fail! " __VA_ARGS__)); \ - std::terminate(); \ - } \ - } while (false) - /// Usage: /// ``` /// RUNTIME_ASSERT(a != b); @@ -286,7 +276,7 @@ inline void log(const char * filename, int lineno, const char * condition, const &__FILE__[LogFmtDetails::getFileNameOffset(__FILE__)], \ __LINE__, \ #condition, \ - __VA_ARGS__); \ + ##__VA_ARGS__); \ std::terminate(); \ } \ } while (false) From 2b49ec434a99aefde75455f58f48a82c939c82b0 Mon Sep 17 00:00:00 2001 From: fuzhe1989 Date: Tue, 26 Jul 2022 01:15:00 +0800 Subject: [PATCH 3/6] update --- dbms/src/Common/Exception.cpp | 5 +++-- dbms/src/Common/Exception.h | 27 +-------------------------- dbms/src/Common/FailPoint.cpp | 4 ++-- dbms/src/Common/FailPoint.h | 2 +- dbms/src/Server/Server.cpp | 4 ++-- 5 files changed, 9 insertions(+), 33 deletions(-) diff --git a/dbms/src/Common/Exception.cpp b/dbms/src/Common/Exception.cpp index e04d8a642c8..1ef2166e032 100644 --- a/dbms/src/Common/Exception.cpp +++ b/dbms/src/Common/Exception.cpp @@ -269,9 +269,10 @@ ExecutionStatus::fromCurrentException(const std::string & start_of_message) namespace exception_details { -Poco::Logger * getDefaultFatalLogger() +const LoggerPtr & getDefaultFatalLogger() { - return &Poco::Logger::get("DefaultFatal"); + static const auto logger = std::make_shared("DefaultFatal", ""); + return logger; } } // namespace exception_details diff --git a/dbms/src/Common/Exception.h b/dbms/src/Common/Exception.h index fd3b30b9c92..0a81dedb3a5 100644 --- a/dbms/src/Common/Exception.h +++ b/dbms/src/Common/Exception.h @@ -184,16 +184,7 @@ inline Poco::Message generateLogMessage(const std::string & logger_name, const c lineno); } -Poco::Logger * getDefaultFatalLogger(); - -inline void log(const char * filename, int lineno, const char * condition, Poco::Logger * logger) -{ - if (likely(logger->fatal())) - { - auto message = generateLogMessage(logger->name(), filename, lineno, condition); - logger->log(message); - } -} +const LoggerPtr & getDefaultFatalLogger(); inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger) { @@ -225,22 +216,6 @@ inline void log(const char * filename, int lineno, const char * condition, const } } -template -inline void log(const char * filename, int lineno, const char * condition, Poco::Logger * logger, const char * fmt_str, Args &&... args) -{ - if (logger->fatal()) - { - auto message = generateLogMessage( - logger->name(), - filename, - lineno, - condition, - fmt_str, - std::forward(args)...); - logger->log(message); - } -} - template inline void log(const char * filename, int lineno, const char * condition, const char * fmt_str, Args &&... args) { diff --git a/dbms/src/Common/FailPoint.cpp b/dbms/src/Common/FailPoint.cpp index ad5010d7826..fe04d68eac8 100644 --- a/dbms/src/Common/FailPoint.cpp +++ b/dbms/src/Common/FailPoint.cpp @@ -227,7 +227,7 @@ void FailPointHelper::wait(const String & fail_point_name) } } -void FailPointHelper::initRandomFailPoints(Poco::Util::LayeredConfiguration & config, Poco::Logger * log) +void FailPointHelper::initRandomFailPoints(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log) { String random_fail_point_cfg = config.getString("flash.random_fail_points", ""); if (random_fail_point_cfg.empty()) @@ -272,7 +272,7 @@ void FailPointHelper::disableFailPoint(const String &) {} void FailPointHelper::wait(const String &) {} -void FailPointHelper::initRandomFailPoints(Poco::Util::LayeredConfiguration &, Poco::Logger *) {} +void FailPointHelper::initRandomFailPoints(Poco::Util::LayeredConfiguration &, const LoggerPtr &) {} void FailPointHelper::enableRandomFailPoint(const String &, double) {} #endif diff --git a/dbms/src/Common/FailPoint.h b/dbms/src/Common/FailPoint.h index 31df2dbdcd2..dc0913fc620 100644 --- a/dbms/src/Common/FailPoint.h +++ b/dbms/src/Common/FailPoint.h @@ -60,7 +60,7 @@ class FailPointHelper * 2. Parse flash.random_fail_points, which expect to has "FailPointA-RatioA,FailPointB-RatioB,..." format * 3. Call enableRandomFailPoint method with parsed FailPointName and Rate */ - static void initRandomFailPoints(Poco::Util::LayeredConfiguration & config, Poco::Logger * log); + static void initRandomFailPoints(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log); static void enableRandomFailPoint(const String & fail_point_name, double rate); diff --git a/dbms/src/Server/Server.cpp b/dbms/src/Server/Server.cpp index fa34400d5e3..06e25a84e92 100644 --- a/dbms/src/Server/Server.cpp +++ b/dbms/src/Server/Server.cpp @@ -344,7 +344,7 @@ struct TCPServer : Poco::Net::TCPServer } }; -void UpdateMallocConfig([[maybe_unused]] Poco::Logger * log) +void UpdateMallocConfig([[maybe_unused]] const LoggerPtr & log) { #ifdef RUN_FAIL_RETURN static_assert(false); @@ -992,7 +992,7 @@ int Server::main(const std::vector & /*args*/) { setThreadName("TiFlashMain"); - Poco::Logger * log = &logger(); + const auto log = std::make_shared(&logger(), ""); #ifdef FIU_ENABLE fiu_init(0); // init failpoint FailPointHelper::initRandomFailPoints(config(), log); From 831314f46bcca46d132ad7e255d9ed876032f08b Mon Sep 17 00:00:00 2001 From: fuzhe1989 Date: Tue, 26 Jul 2022 08:52:53 +0800 Subject: [PATCH 4/6] update --- dbms/src/Common/TiFlashSecurity.h | 2 +- dbms/src/Encryption/RateLimiter.cpp | 6 ++--- dbms/src/Encryption/RateLimiter.h | 16 +++++++++----- dbms/src/Functions/FunctionsString.cpp | 2 +- dbms/src/Server/RaftConfigParser.cpp | 2 +- dbms/src/Server/RaftConfigParser.h | 2 +- dbms/src/Server/Server.cpp | 22 +++++++++---------- dbms/src/Server/StorageConfigParser.cpp | 10 ++++----- dbms/src/Server/StorageConfigParser.h | 11 +++++----- dbms/src/Server/UserConfigParser.cpp | 2 +- dbms/src/Server/UserConfigParser.h | 3 ++- .../src/Server/tests/gtest_storage_config.cpp | 8 +++---- 12 files changed, 47 insertions(+), 39 deletions(-) diff --git a/dbms/src/Common/TiFlashSecurity.h b/dbms/src/Common/TiFlashSecurity.h index 8dde3fe5a98..56b98b5b02e 100644 --- a/dbms/src/Common/TiFlashSecurity.h +++ b/dbms/src/Common/TiFlashSecurity.h @@ -54,7 +54,7 @@ struct TiFlashSecurityConfig public: TiFlashSecurityConfig() = default; - TiFlashSecurityConfig(Poco::Util::LayeredConfiguration & config, Poco::Logger * log) + TiFlashSecurityConfig(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log) { if (config.has("security")) { diff --git a/dbms/src/Encryption/RateLimiter.cpp b/dbms/src/Encryption/RateLimiter.cpp index 38fd8468341..8c986f93f71 100644 --- a/dbms/src/Encryption/RateLimiter.cpp +++ b/dbms/src/Encryption/RateLimiter.cpp @@ -298,7 +298,7 @@ ReadLimiter::ReadLimiter( , getIOStatistic(std::move(getIOStatistic_)) , last_stat_bytes(getIOStatistic()) , last_stat_time(now()) - , log(&Poco::Logger::get("ReadLimiter")) + , log(Logger::get("ReadLimiter")) , get_io_statistic_period_us(get_io_stat_period_us) {} @@ -382,7 +382,7 @@ void ReadLimiter::refillAndAlloc() } IORateLimiter::IORateLimiter() - : log(&Poco::Logger::get("IORateLimiter")) + : log(Logger::get("IORateLimiter")) , stop(false) {} @@ -693,7 +693,7 @@ IOLimitTuner::IOLimitTuner( , bg_read_stat(std::move(bg_read_stat_)) , fg_read_stat(std::move(fg_read_stat_)) , io_config(io_config_) - , log(&Poco::Logger::get("IOLimitTuner")) + , log(Logger::get("IOLimitTuner")) {} IOLimitTuner::TuneResult IOLimitTuner::tune() const diff --git a/dbms/src/Encryption/RateLimiter.h b/dbms/src/Encryption/RateLimiter.h index f44beeb8ed7..9d29ea10e0a 100644 --- a/dbms/src/Encryption/RateLimiter.h +++ b/dbms/src/Encryption/RateLimiter.h @@ -185,7 +185,7 @@ class ReadLimiter : public WriteLimiter return std::chrono::time_point_cast(std::chrono::system_clock::now()); } TimePoint last_stat_time; - Poco::Logger * log; + LoggerPtr log; Int64 get_io_statistic_period_us; }; @@ -260,7 +260,7 @@ class IORateLimiter std::vector bg_thread_ids; IOInfo last_io_info; - Poco::Logger * log; + LoggerPtr log; std::atomic stop; std::thread auto_tune_thread; @@ -419,8 +419,14 @@ class IOLimitTuner High = 3, Emergency = 4 }; - Watermark writeWatermark() const { return getWatermark(writePct()); } - Watermark readWatermark() const { return getWatermark(readPct()); } + Watermark writeWatermark() const + { + return getWatermark(writePct()); + } + Watermark readWatermark() const + { + return getWatermark(readPct()); + } Watermark getWatermark(int pct) const; // Returns @@ -475,6 +481,6 @@ class IOLimitTuner LimiterStatUPtr bg_read_stat; LimiterStatUPtr fg_read_stat; StorageIORateLimitConfig io_config; - Poco::Logger * log; + LoggerPtr log; }; } // namespace DB diff --git a/dbms/src/Functions/FunctionsString.cpp b/dbms/src/Functions/FunctionsString.cpp index ef8583b4f93..b7e69d92a16 100644 --- a/dbms/src/Functions/FunctionsString.cpp +++ b/dbms/src/Functions/FunctionsString.cpp @@ -5286,7 +5286,7 @@ class FunctionHexInt : public IFunction { UInt64 number = col->getUInt(i); - int print_size = sprintf(reinterpret_cast(&res_chars[prev_res_offset]), "%lX", number); + int print_size = sprintf(reinterpret_cast(&res_chars[prev_res_offset]), "%llX", number); res_chars[prev_res_offset + print_size] = 0; // Add the size of printed string and a tailing zero prev_res_offset += print_size + 1; diff --git a/dbms/src/Server/RaftConfigParser.cpp b/dbms/src/Server/RaftConfigParser.cpp index 2f0a88855cd..39730636431 100644 --- a/dbms/src/Server/RaftConfigParser.cpp +++ b/dbms/src/Server/RaftConfigParser.cpp @@ -30,7 +30,7 @@ extern const int INVALID_CONFIG_PARAMETER; } // namespace ErrorCodes /// Load raft related configs. -TiFlashRaftConfig TiFlashRaftConfig::parseSettings(Poco::Util::LayeredConfiguration & config, Poco::Logger * log) +TiFlashRaftConfig TiFlashRaftConfig::parseSettings(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log) { TiFlashRaftConfig res; res.flash_server_addr = config.getString("flash.service_addr", "0.0.0.0:3930"); diff --git a/dbms/src/Server/RaftConfigParser.h b/dbms/src/Server/RaftConfigParser.h index 0eb78ba20a8..c42304289e2 100644 --- a/dbms/src/Server/RaftConfigParser.h +++ b/dbms/src/Server/RaftConfigParser.h @@ -46,7 +46,7 @@ struct TiFlashRaftConfig public: TiFlashRaftConfig() = default; - static TiFlashRaftConfig parseSettings(Poco::Util::LayeredConfiguration & config, Poco::Logger * log); + static TiFlashRaftConfig parseSettings(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log); }; } // namespace DB diff --git a/dbms/src/Server/Server.cpp b/dbms/src/Server/Server.cpp index 06e25a84e92..7fa87cf85f0 100644 --- a/dbms/src/Server/Server.cpp +++ b/dbms/src/Server/Server.cpp @@ -157,7 +157,7 @@ void loadMiConfig(Logger * log) namespace { -[[maybe_unused]] void tryLoadBoolConfigFromEnv(Poco::Logger * log, bool & target, const char * name) +[[maybe_unused]] void tryLoadBoolConfigFromEnv(const DB::LoggerPtr & log, bool & target, const char * name) { auto * config = getenv(name); if (config) @@ -297,7 +297,7 @@ pingcap::ClusterConfig getClusterConfig(const TiFlashSecurityConfig & security_c return config; } -Poco::Logger * grpc_log = nullptr; +LoggerPtr grpc_log; void printGRPCLog(gpr_log_func_args * args) { @@ -435,7 +435,7 @@ struct RaftStoreProxyRunner : boost::noncopyable size_t stack_size = 1024 * 1024 * 20; }; - RaftStoreProxyRunner(RunRaftStoreProxyParms && parms_, Poco::Logger * log_) + RaftStoreProxyRunner(RunRaftStoreProxyParms && parms_, const LoggerPtr & log_) : parms(std::move(parms_)) , log(log_) {} @@ -470,11 +470,11 @@ struct RaftStoreProxyRunner : boost::noncopyable RunRaftStoreProxyParms parms; pthread_t thread{}; - Poco::Logger * log; + const LoggerPtr & log; }; // We only need this task run once. -void initStores(Context & global_context, Poco::Logger * log, bool lazily_init_store) +void initStores(Context & global_context, const LoggerPtr & log, bool lazily_init_store) { auto do_init_stores = [&global_context, log]() { auto storages = global_context.getTMTContext().getStorages().getAllStorage(); @@ -518,7 +518,7 @@ void initStores(Context & global_context, Poco::Logger * log, bool lazily_init_s } } -void handleRpcs(grpc::ServerCompletionQueue * curcq, Poco::Logger * log) +void handleRpcs(grpc::ServerCompletionQueue * curcq, const LoggerPtr & log) { GET_METRIC(tiflash_thread_count, type_total_rpc_async_worker).Increment(); SCOPE_EXIT({ @@ -579,7 +579,7 @@ void handleRpcs(grpc::ServerCompletionQueue * curcq, Poco::Logger * log) class Server::FlashGrpcServerHolder { public: - FlashGrpcServerHolder(Server & server, const TiFlashRaftConfig & raft_config, Poco::Logger * log_) + FlashGrpcServerHolder(Server & server, const TiFlashRaftConfig & raft_config, const LoggerPtr & log_) : log(log_) , is_shutdown(std::make_shared>(false)) { @@ -695,7 +695,7 @@ class Server::FlashGrpcServerHolder } private: - Poco::Logger * log; + const LoggerPtr & log; std::shared_ptr> is_shutdown; std::unique_ptr flash_service = nullptr; std::unique_ptr diagnostics_service = nullptr; @@ -709,7 +709,7 @@ class Server::FlashGrpcServerHolder class Server::TcpHttpServersHolder { public: - TcpHttpServersHolder(Server & server_, const Settings & settings, Poco::Logger * log_) + TcpHttpServersHolder(Server & server_, const Settings & settings, const LoggerPtr & log_) : server(server_) , log(log_) , server_pool(1, server.config().getUInt("max_connections", 1024)) @@ -983,7 +983,7 @@ class Server::TcpHttpServersHolder private: Server & server; - Poco::Logger * log; + const LoggerPtr & log; Poco::ThreadPool server_pool; std::vector> servers; }; @@ -1079,7 +1079,7 @@ int Server::main(const std::vector & /*args*/) } // print necessary grpc log. - grpc_log = &Poco::Logger::get("grpc"); + grpc_log = Logger::get("grpc"); gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); gpr_set_log_function(&printGRPCLog); diff --git a/dbms/src/Server/StorageConfigParser.cpp b/dbms/src/Server/StorageConfigParser.cpp index d43ccb850f1..09b7807c397 100644 --- a/dbms/src/Server/StorageConfigParser.cpp +++ b/dbms/src/Server/StorageConfigParser.cpp @@ -61,7 +61,7 @@ static String getNormalizedPath(const String & s) return getCanonicalPath(Poco::Path{s}.toString()); } -void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger * log) +void TiFlashStorageConfig::parseStoragePath(const String & storage, const LoggerPtr & log) { std::istringstream ss(storage); cpptoml::parser p(ss); @@ -181,7 +181,7 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger } } -void TiFlashStorageConfig::parseMisc(const String & storage_section, Poco::Logger * log) +void TiFlashStorageConfig::parseMisc(const String & storage_section, const LoggerPtr & log) { std::istringstream ss(storage_section); cpptoml::parser p(ss); @@ -233,7 +233,7 @@ Strings TiFlashStorageConfig::getAllNormalPaths() const return all_normal_path; } -bool TiFlashStorageConfig::parseFromDeprecatedConfiguration(Poco::Util::LayeredConfiguration & config, Poco::Logger * log) +bool TiFlashStorageConfig::parseFromDeprecatedConfiguration(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log) { if (!config.has("path")) return false; @@ -302,7 +302,7 @@ bool TiFlashStorageConfig::parseFromDeprecatedConfiguration(Poco::Util::LayeredC return true; } -std::tuple TiFlashStorageConfig::parseSettings(Poco::Util::LayeredConfiguration & config, Poco::Logger * log) +std::tuple TiFlashStorageConfig::parseSettings(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log) { size_t global_capacity_quota = 0; // "0" by default, means no quota, use the whole disk capacity. TiFlashStorageConfig storage_config; @@ -379,7 +379,7 @@ std::tuple TiFlashStorageConfig::parseSettings(Poc return std::make_tuple(global_capacity_quota, storage_config); } -void StorageIORateLimitConfig::parse(const String & storage_io_rate_limit, Poco::Logger * log) +void StorageIORateLimitConfig::parse(const String & storage_io_rate_limit, const LoggerPtr & log) { std::istringstream ss(storage_io_rate_limit); cpptoml::parser p(ss); diff --git a/dbms/src/Server/StorageConfigParser.h b/dbms/src/Server/StorageConfigParser.h index 4efc5637634..f3a779d2f63 100644 --- a/dbms/src/Server/StorageConfigParser.h +++ b/dbms/src/Server/StorageConfigParser.h @@ -14,6 +14,7 @@ #pragma once +#include #include #include @@ -75,7 +76,7 @@ struct StorageIORateLimitConfig , auto_tune_sec(5) {} - void parse(const String & storage_io_rate_limit, Poco::Logger * log); + void parse(const String & storage_io_rate_limit, const LoggerPtr & log); std::string toString() const; @@ -109,14 +110,14 @@ struct TiFlashStorageConfig Strings getAllNormalPaths() const; - static std::tuple parseSettings(Poco::Util::LayeredConfiguration & config, Poco::Logger * log); + static std::tuple parseSettings(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log); private: - void parseStoragePath(const String & storage_section, Poco::Logger * log); + void parseStoragePath(const String & storage_section, const LoggerPtr & log); - bool parseFromDeprecatedConfiguration(Poco::Util::LayeredConfiguration & config, Poco::Logger * log); + bool parseFromDeprecatedConfiguration(Poco::Util::LayeredConfiguration & config, const LoggerPtr & log); - void parseMisc(const String & storage_section, Poco::Logger * log); + void parseMisc(const String & storage_section, const LoggerPtr & log); }; diff --git a/dbms/src/Server/UserConfigParser.cpp b/dbms/src/Server/UserConfigParser.cpp index 9d17ce4628c..21b5ea67b3e 100644 --- a/dbms/src/Server/UserConfigParser.cpp +++ b/dbms/src/Server/UserConfigParser.cpp @@ -50,7 +50,7 @@ ConfigReloaderPtr parseSettings( Poco::Util::LayeredConfiguration & config, const std::string & config_path, std::unique_ptr & global_context, - Poco::Logger * log) + const LoggerPtr & log) { std::string users_config_path = config.getString("users_config", String(1, '\0')); bool load_from_main_config_path = true; diff --git a/dbms/src/Server/UserConfigParser.h b/dbms/src/Server/UserConfigParser.h index 17e2f6a7029..395cf71b3ed 100644 --- a/dbms/src/Server/UserConfigParser.h +++ b/dbms/src/Server/UserConfigParser.h @@ -14,6 +14,7 @@ #pragma once +#include #include #include @@ -38,7 +39,7 @@ ConfigReloaderPtr parseSettings( Poco::Util::LayeredConfiguration & config, const std::string & config_path, std::unique_ptr & global_context, - Poco::Logger * log); + const LoggerPtr & log); } } // namespace DB diff --git a/dbms/src/Server/tests/gtest_storage_config.cpp b/dbms/src/Server/tests/gtest_storage_config.cpp index 9f6b64526f4..bfccabe304c 100644 --- a/dbms/src/Server/tests/gtest_storage_config.cpp +++ b/dbms/src/Server/tests/gtest_storage_config.cpp @@ -37,13 +37,13 @@ class StorageConfigTest : public ::testing::Test { public: StorageConfigTest() - : log(&Poco::Logger::get("StorageConfigTest")) + : log(Logger::get("StorageConfigTest")) {} static void SetUpTestCase() {} protected: - Poco::Logger * log; + LoggerPtr log; }; TEST_F(StorageConfigTest, SimpleSinglePath) @@ -449,7 +449,7 @@ dir=["/data0/tiflash"] capacity=[ 1024 ] )", }; - Poco::Logger * log = &Poco::Logger::get("PathCapacityMetrics_test"); + auto log = Logger::get("PathCapacityMetrics_test"); for (size_t i = 0; i < tests.size(); ++i) { @@ -603,7 +603,7 @@ background_read_weight=2 )", }; - Poco::Logger * log = &Poco::Logger::get("StorageIORateLimitConfigTest"); + auto log = Logger::get("StorageIORateLimitConfigTest"); auto verify_default = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 0); From 02c3824e909b8f76527b607561c2ebbdc6074fbe Mon Sep 17 00:00:00 2001 From: fuzhe1989 Date: Tue, 26 Jul 2022 08:53:32 +0800 Subject: [PATCH 5/6] update --- dbms/src/Functions/FunctionsString.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Functions/FunctionsString.cpp b/dbms/src/Functions/FunctionsString.cpp index b7e69d92a16..ef8583b4f93 100644 --- a/dbms/src/Functions/FunctionsString.cpp +++ b/dbms/src/Functions/FunctionsString.cpp @@ -5286,7 +5286,7 @@ class FunctionHexInt : public IFunction { UInt64 number = col->getUInt(i); - int print_size = sprintf(reinterpret_cast(&res_chars[prev_res_offset]), "%llX", number); + int print_size = sprintf(reinterpret_cast(&res_chars[prev_res_offset]), "%lX", number); res_chars[prev_res_offset + print_size] = 0; // Add the size of printed string and a tailing zero prev_res_offset += print_size + 1; From 709cd7151e704afd14c09f0f7e72e17c69cf6bd3 Mon Sep 17 00:00:00 2001 From: fuzhe1989 Date: Tue, 26 Jul 2022 12:34:45 +0800 Subject: [PATCH 6/6] update Signed-off-by: fuzhe1989 --- dbms/src/Common/Exception.h | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/dbms/src/Common/Exception.h b/dbms/src/Common/Exception.h index 0a81dedb3a5..2886ae4d5ec 100644 --- a/dbms/src/Common/Exception.h +++ b/dbms/src/Common/Exception.h @@ -167,8 +167,8 @@ inline std::string generateFormattedMessage(const char * condition) return fmt::format("Assert {} fail!", condition); } -template -inline std::string generateFormattedMessage(const char * condition, T && fmt_str, Args &&... args) +template +inline std::string generateFormattedMessage(const char * condition, const char * fmt_str, Args &&... args) { return FmtBuffer().fmtAppend("Assert {} fail! ", condition).fmtAppend(fmt_str, std::forward(args)...).toString(); } @@ -186,22 +186,8 @@ inline Poco::Message generateLogMessage(const std::string & logger_name, const c const LoggerPtr & getDefaultFatalLogger(); -inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger) -{ - if (likely(logger->fatal())) - { - auto message = generateLogMessage(logger->name(), filename, lineno, condition); - logger->log(message); - } -} - -inline void log(const char * filename, int lineno, const char * condition) -{ - log(filename, lineno, condition, getDefaultFatalLogger()); -} - template -inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger, const char * fmt_str, Args &&... args) +inline void log(const char * filename, int lineno, const char * condition, const LoggerPtr & logger, Args &&... args) { if (logger->fatal()) { @@ -210,12 +196,16 @@ inline void log(const char * filename, int lineno, const char * condition, const filename, lineno, condition, - fmt_str, std::forward(args)...); logger->log(message); } } +inline void log(const char * filename, int lineno, const char * condition) +{ + log(filename, lineno, condition, getDefaultFatalLogger()); +} + template inline void log(const char * filename, int lineno, const char * condition, const char * fmt_str, Args &&... args) {