This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 10a54350d fix: support core dump for fatal logging in
simple_logger::dsn_logv and call fatal logging exactly once for the assertion
(#1580)
10a54350d is described below
commit 10a54350de09fdfb1720c3d50478b4cd8989932b
Author: Dan Wang <[email protected]>
AuthorDate: Tue Aug 8 11:54:17 2023 +0800
fix: support core dump for fatal logging in simple_logger::dsn_logv and
call fatal logging exactly once for the assertion (#1580)
https://github.com/apache/incubator-pegasus/issues/1579
Following fixes are included in this PR to solve the problems:
- support core dump for fatal logging in `simple_logger::dsn_logv`;
- call fatal logging exactly once for the assertion macros `CHECK*`;
- add fail point to keep the logging test running without termination.
---
src/client/replication_ddl_client.cpp | 1 -
src/meta/test/backup_test.cpp | 1 +
src/server/result_writer.cpp | 1 +
src/utils/fmt_logging.h | 6 +++---
src/utils/simple_logger.cpp | 35 +++++++++++++++++++++++++++++------
src/utils/simple_logger.h | 24 +++++++++++++-----------
src/utils/test/logging.cpp | 14 ++++++++++++--
7 files changed, 59 insertions(+), 23 deletions(-)
diff --git a/src/client/replication_ddl_client.cpp
b/src/client/replication_ddl_client.cpp
index 00276e986..ff0b3269c 100644
--- a/src/client/replication_ddl_client.cpp
+++ b/src/client/replication_ddl_client.cpp
@@ -45,7 +45,6 @@
#include "common/replication.codes.h"
#include "common/replication_common.h"
#include "common/replication_enums.h"
-#include "fmt/core.h"
#include "fmt/ostream.h"
#include "meta/meta_rpc_types.h"
#include "runtime/api_layer1.h"
diff --git a/src/meta/test/backup_test.cpp b/src/meta/test/backup_test.cpp
index 70062ba69..35b88bb97 100644
--- a/src/meta/test/backup_test.cpp
+++ b/src/meta/test/backup_test.cpp
@@ -29,6 +29,7 @@
#include <set>
#include <string>
#include <thread>
+#include <type_traits>
#include <utility>
#include <vector>
diff --git a/src/server/result_writer.cpp b/src/server/result_writer.cpp
index d4e9ebeb7..16ec2886b 100644
--- a/src/server/result_writer.cpp
+++ b/src/server/result_writer.cpp
@@ -21,6 +21,7 @@
#include <pegasus/error.h>
#include <chrono>
+#include <type_traits>
#include <utility>
#include "pegasus/client.h"
diff --git a/src/utils/fmt_logging.h b/src/utils/fmt_logging.h
index 1f6e96120..f0c74bd29 100644
--- a/src/utils/fmt_logging.h
+++ b/src/utils/fmt_logging.h
@@ -57,9 +57,9 @@
#define CHECK_EXPRESSION(expression, evaluation, ...)
\
do {
\
if (dsn_unlikely(!(evaluation))) {
\
- dlog_f(LOG_LEVEL_FATAL, "assertion expression: " #expression);
\
- dlog_f(LOG_LEVEL_FATAL, __VA_ARGS__);
\
- dsn_coredump();
\
+ std::string assertion_info("assertion expression: [" #expression
"] "); \
+ assertion_info += fmt::format(__VA_ARGS__);
\
+ LOG_FATAL(assertion_info);
\
}
\
} while (false)
diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp
index 53fc3a889..98b1693d1 100644
--- a/src/utils/simple_logger.cpp
+++ b/src/utils/simple_logger.cpp
@@ -38,11 +38,14 @@
#include "runtime/api_layer1.h"
#include "runtime/task/task_spec.h"
#include "utils/command_manager.h"
+#include "utils/fail_point.h"
#include "utils/filesystem.h"
#include "utils/flags.h"
#include "utils/fmt_logging.h"
#include "utils/ports.h"
#include "utils/process_utils.h"
+#include "utils/string_conv.h"
+#include "utils/string_view.h"
#include "utils/strings.h"
#include "utils/time_utils.h"
@@ -94,6 +97,28 @@ static void print_header(FILE *fp, dsn_log_level_t log_level)
log_prefixed_message_func().c_str());
}
+namespace {
+
+inline void process_fatal_log(dsn_log_level_t log_level)
+{
+ if (dsn_likely(log_level < LOG_LEVEL_FATAL)) {
+ return;
+ }
+
+ bool coredump = true;
+ FAIL_POINT_INJECT_NOT_RETURN_F("coredump_for_fatal_log",
[&coredump](dsn::string_view str) {
+ CHECK(buf2bool(str, coredump),
+ "invalid coredump toggle for fatal log, should be true or false:
{}",
+ str);
+ });
+
+ if (dsn_likely(coredump)) {
+ dsn_coredump();
+ }
+}
+
+} // anonymous namespace
+
screen_logger::screen_logger(bool short_header) { _short_header =
short_header; }
screen_logger::~screen_logger(void) {}
@@ -114,9 +139,7 @@ void screen_logger::dsn_logv(const char *file,
vprintf(fmt, args);
printf("\n");
- if (dsn_unlikely(log_level >= LOG_LEVEL_FATAL)) {
- dsn_coredump();
- }
+ process_fatal_log(log_level);
}
void screen_logger::flush() { ::fflush(stdout); }
@@ -262,6 +285,8 @@ void simple_logger::dsn_logv(const char *file,
printf("\n");
}
+ process_fatal_log(log_level);
+
if (++_lines >= 200000) {
create_log_file();
}
@@ -292,9 +317,7 @@ void simple_logger::dsn_log(const char *file,
printf("%s\n", str);
}
- if (dsn_unlikely(log_level >= LOG_LEVEL_FATAL)) {
- dsn_coredump();
- }
+ process_fatal_log(log_level);
if (++_lines >= 200000) {
create_log_file();
diff --git a/src/utils/simple_logger.h b/src/utils/simple_logger.h
index 74a44d2dd..71535b26f 100644
--- a/src/utils/simple_logger.h
+++ b/src/utils/simple_logger.h
@@ -46,18 +46,20 @@ public:
explicit screen_logger(bool short_header);
~screen_logger() override;
- virtual void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args);
+ void dsn_logv(const char *file,
+ const char *function,
+ const int line,
+ dsn_log_level_t log_level,
+ const char *fmt,
+ va_list args) override;
- virtual void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str){};
+ void dsn_log(const char *file,
+ const char *function,
+ const int line,
+ dsn_log_level_t log_level,
+ const char *str) override
+ {
+ }
virtual void flush();
diff --git a/src/utils/test/logging.cpp b/src/utils/test/logging.cpp
index f526dc5a2..eb3170d2a 100644
--- a/src/utils/test/logging.cpp
+++ b/src/utils/test/logging.cpp
@@ -38,6 +38,7 @@
#include <string>
#include "utils/api_utilities.h"
+#include "utils/fail_point.h"
#include "utils/fmt_logging.h"
TEST(core, logging)
@@ -63,7 +64,7 @@ TEST(core, logging_big_log)
big_str.c_str());
}
-TEST(core, dlog_f)
+TEST(core, dlog)
{
struct test_case
{
@@ -76,7 +77,16 @@ TEST(core, dlog_f)
{dsn_log_level_t::LOG_LEVEL_ERROR, "\\x00%d\\x00\\x01%n/nm"},
{dsn_log_level_t::LOG_LEVEL_FATAL, "\\x00%d\\x00\\x01%n/nm"}};
+ dsn::fail::setup();
+ dsn::fail::cfg("coredump_for_fatal_log", "void(false)");
+
for (auto test : tests) {
- dlog_f(test.level, "sortkey = {}", test.str);
+ // Test logging_provider::dsn_log
+ dlog_f(test.level, "dlog_f: sortkey = {}", test.str);
+
+ // Test logging_provider::dsn_logv
+ dlog(test.level, "dlog: sortkey = %s", test.str.c_str());
}
+
+ dsn::fail::teardown();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
