From a080ac403f6e17363c9d48645bdaf36138bbb2fa Mon Sep 17 00:00:00 2001 From: jiashuo Date: Tue, 8 Feb 2022 19:03:49 +0800 Subject: [PATCH 1/7] init --- include/dsn/utility/fail_point.h | 6 +++--- src/aio/native_linux_aio_provider.cpp | 2 +- src/aio/test/aio.cpp | 4 ++-- src/replica/duplication/load_from_private_log.cpp | 2 +- src/replica/duplication/test/load_from_private_log_test.cpp | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/dsn/utility/fail_point.h b/include/dsn/utility/fail_point.h index d2c100db87..af3d1e15ac 100644 --- a/include/dsn/utility/fail_point.h +++ b/include/dsn/utility/fail_point.h @@ -50,14 +50,14 @@ /// The only entry to define a fail point with `void` function: lambda function must be /// return void type. When a fail point is defined, it's referenced via the name. -#define FAIL_POINT_INJECT_VOID_F(name, lambda) \ +#define FAIL_POINT_INJECT_NOT_RETURN_F(name, lambda) \ do { \ if (dsn_likely(!::dsn::fail::_S_FAIL_POINT_ENABLED)) \ break; \ auto __Func = lambda; \ auto __Res = ::dsn::fail::eval(name); \ - if (__Res == nullptr) { \ - __Func(); \ + if (__Res != nullptr) { \ + __Func(*__Res); \ } \ } while (0) diff --git a/src/aio/native_linux_aio_provider.cpp b/src/aio/native_linux_aio_provider.cpp index 184029a05b..6f7be7f2d7 100644 --- a/src/aio/native_linux_aio_provider.cpp +++ b/src/aio/native_linux_aio_provider.cpp @@ -93,7 +93,7 @@ error_code native_linux_aio_provider::write(const aio_context &aio_ctx, } // mock the `ret` to reproduce the `write incomplete` case in the first write - FAIL_POINT_INJECT_VOID_F("aio_pwrite_incomplete", [&]() -> void { + FAIL_POINT_INJECT_NOT_RETURN_F("aio_pwrite_incomplete", [&](string_view s) -> void { if (dsn_unlikely(buffer_offset == 0)) { --ret; } diff --git a/src/aio/test/aio.cpp b/src/aio/test/aio.cpp index 0b62352387..6291f44eb4 100644 --- a/src/aio/test/aio.cpp +++ b/src/aio/test/aio.cpp @@ -39,7 +39,7 @@ DEFINE_TASK_CODE_AIO(LPC_AIO_TEST, TASK_PRIORITY_COMMON, THREAD_POOL_TEST_SERVER TEST(core, aio) { fail::setup(); - fail::cfg("aio_pwrite_incomplete", "off()"); + fail::cfg("aio_pwrite_incomplete", "return()"); const char *buffer = "hello, world"; int len = (int)strlen(buffer); @@ -148,7 +148,7 @@ TEST(core, aio_share) TEST(core, operation_failed) { fail::setup(); - fail::cfg("aio_pwrite_incomplete", "off()"); + fail::cfg("aio_pwrite_incomplete", "return()"); auto fp = file::open("tmp_test_file", O_WRONLY, 0600); EXPECT_TRUE(fp == nullptr); diff --git a/src/replica/duplication/load_from_private_log.cpp b/src/replica/duplication/load_from_private_log.cpp index 51286ea897..28b0e22e62 100644 --- a/src/replica/duplication/load_from_private_log.cpp +++ b/src/replica/duplication/load_from_private_log.cpp @@ -83,7 +83,7 @@ void load_from_private_log::run() _duplicator->progress().confirmed_decree); repeat(1_s); - FAIL_POINT_INJECT_VOID_F("duplication_sync_complete", [&]() -> void { + FAIL_POINT_INJECT_NOT_RETURN_F("duplication_sync_complete", [&](string_view s) -> void { if (_duplicator->progress().confirmed_decree == invalid_decree) { // set_confirmed_decree(9), the value must be equal (decree_start of // `test_start_duplication` in `load_from_private_log_test.cpp`) -1 diff --git a/src/replica/duplication/test/load_from_private_log_test.cpp b/src/replica/duplication/test/load_from_private_log_test.cpp index 122d79e7b2..5d809749ea 100644 --- a/src/replica/duplication/test/load_from_private_log_test.cpp +++ b/src/replica/duplication/test/load_from_private_log_test.cpp @@ -175,7 +175,7 @@ class load_from_private_log_test : public duplication_test_base fail::setup(); fail::cfg("open_read", "25%1*return()"); fail::cfg("mutation_log_read_log_block", "25%1*return()"); - fail::cfg("duplication_sync_complete", "off()"); + fail::cfg("duplication_sync_complete", "return()"); duplicator->run_pipeline(); duplicator->wait_all(); fail::teardown(); From 954f4bec7df7a066b03599976891f5265e80ed37 Mon Sep 17 00:00:00 2001 From: jiashuo Date: Tue, 8 Feb 2022 19:13:57 +0800 Subject: [PATCH 2/7] init --- src/aio/test/aio.cpp | 4 ++-- src/replica/duplication/test/load_from_private_log_test.cpp | 2 +- src/utils/fail_point.cpp | 5 +++++ src/utils/fail_point_impl.h | 1 + 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/aio/test/aio.cpp b/src/aio/test/aio.cpp index 6291f44eb4..d739178d13 100644 --- a/src/aio/test/aio.cpp +++ b/src/aio/test/aio.cpp @@ -39,7 +39,7 @@ DEFINE_TASK_CODE_AIO(LPC_AIO_TEST, TASK_PRIORITY_COMMON, THREAD_POOL_TEST_SERVER TEST(core, aio) { fail::setup(); - fail::cfg("aio_pwrite_incomplete", "return()"); + fail::cfg("aio_pwrite_incomplete", "void()"); const char *buffer = "hello, world"; int len = (int)strlen(buffer); @@ -148,7 +148,7 @@ TEST(core, aio_share) TEST(core, operation_failed) { fail::setup(); - fail::cfg("aio_pwrite_incomplete", "return()"); + fail::cfg("aio_pwrite_incomplete", "void()"); auto fp = file::open("tmp_test_file", O_WRONLY, 0600); EXPECT_TRUE(fp == nullptr); diff --git a/src/replica/duplication/test/load_from_private_log_test.cpp b/src/replica/duplication/test/load_from_private_log_test.cpp index 5d809749ea..e6c471806a 100644 --- a/src/replica/duplication/test/load_from_private_log_test.cpp +++ b/src/replica/duplication/test/load_from_private_log_test.cpp @@ -175,7 +175,7 @@ class load_from_private_log_test : public duplication_test_base fail::setup(); fail::cfg("open_read", "25%1*return()"); fail::cfg("mutation_log_read_log_block", "25%1*return()"); - fail::cfg("duplication_sync_complete", "return()"); + fail::cfg("duplication_sync_complete", "void()"); duplicator->run_pipeline(); duplicator->wait_all(); fail::teardown(); diff --git a/src/utils/fail_point.cpp b/src/utils/fail_point.cpp index cc262ed69c..d71addbfa6 100644 --- a/src/utils/fail_point.cpp +++ b/src/utils/fail_point.cpp @@ -59,6 +59,8 @@ inline const char *task_type_to_string(fail_point::task_type t) return "Return"; case fail_point::Print: return "Print"; + case fail_point::Void: + return "Void"; default: dfatal("unexpected type: %d", t); __builtin_unreachable(); @@ -123,6 +125,8 @@ bool fail_point::parse_from_string(string_view action) _task = Return; } else if (task_type.compare("print") == 0) { _task = Print; + } else if (task_type.compare("void") == 0) { + _task = Void; } else { return false; } @@ -153,6 +157,7 @@ const std::string *fail_point::eval() switch (_task) { case Off: break; + case Void: case Return: return &_arg; case Print: diff --git a/src/utils/fail_point_impl.h b/src/utils/fail_point_impl.h index fbf3071758..3e8d648d8a 100644 --- a/src/utils/fail_point_impl.h +++ b/src/utils/fail_point_impl.h @@ -47,6 +47,7 @@ struct fail_point Off, Return, Print, + Void, }; void set_action(string_view action); From 0856f02193469b31fd8967101cc3ac222ad3ae1e Mon Sep 17 00:00:00 2001 From: jiashuo Date: Tue, 8 Feb 2022 19:22:56 +0800 Subject: [PATCH 3/7] init --- include/dsn/utility/fail_point.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dsn/utility/fail_point.h b/include/dsn/utility/fail_point.h index af3d1e15ac..a2824dfad1 100644 --- a/include/dsn/utility/fail_point.h +++ b/include/dsn/utility/fail_point.h @@ -48,7 +48,7 @@ } \ } while (0) -/// The only entry to define a fail point with `void` function: lambda function must be +/// The only entry to define a fail point with `no return` function: lambda function usually is /// return void type. When a fail point is defined, it's referenced via the name. #define FAIL_POINT_INJECT_NOT_RETURN_F(name, lambda) \ do { \ From d3e7feade2cd72a2a84613046134ed02a477ec76 Mon Sep 17 00:00:00 2001 From: jiashuo Date: Wed, 9 Feb 2022 10:18:57 +0800 Subject: [PATCH 4/7] init --- include/dsn/utility/fail_point.h | 2 +- src/utils/fail_point.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/dsn/utility/fail_point.h b/include/dsn/utility/fail_point.h index a2824dfad1..7bf9f9c696 100644 --- a/include/dsn/utility/fail_point.h +++ b/include/dsn/utility/fail_point.h @@ -48,7 +48,7 @@ } \ } while (0) -/// The only entry to define a fail point with `no return` function: lambda function usually is +/// The only entry to define a fail point with `not return` function: lambda function usually /// return void type. When a fail point is defined, it's referenced via the name. #define FAIL_POINT_INJECT_NOT_RETURN_F(name, lambda) \ do { \ diff --git a/src/utils/fail_point.cpp b/src/utils/fail_point.cpp index d71addbfa6..9770660635 100644 --- a/src/utils/fail_point.cpp +++ b/src/utils/fail_point.cpp @@ -53,12 +53,20 @@ static fail_point_registry REGISTRY; inline const char *task_type_to_string(fail_point::task_type t) { switch (t) { + // `action` contain `off()`, which would `close` the fail_point whose `function` passed will not + // be executed; case fail_point::Off: return "Off"; + // `action` contain `return()`, which would `return` args passed and execute `return` type + // function passed. it's usually used for `FAIL_POINT_INJECT_F` case fail_point::Return: return "Return"; + // `action` contain `print()`, which would only just print `action` string value and ignore the + // `function` passed case fail_point::Print: return "Print"; + // `action` contain `void()`, which would return args and execute the void type `function` + // passed, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to avoid `return` function case fail_point::Void: return "Void"; default: From 8af2740b391186d28d48d8485079940305b6da14 Mon Sep 17 00:00:00 2001 From: jiashuo Date: Wed, 9 Feb 2022 10:22:15 +0800 Subject: [PATCH 5/7] init --- src/utils/fail_point.cpp | 8 -------- src/utils/fail_point_impl.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/utils/fail_point.cpp b/src/utils/fail_point.cpp index 9770660635..d71addbfa6 100644 --- a/src/utils/fail_point.cpp +++ b/src/utils/fail_point.cpp @@ -53,20 +53,12 @@ static fail_point_registry REGISTRY; inline const char *task_type_to_string(fail_point::task_type t) { switch (t) { - // `action` contain `off()`, which would `close` the fail_point whose `function` passed will not - // be executed; case fail_point::Off: return "Off"; - // `action` contain `return()`, which would `return` args passed and execute `return` type - // function passed. it's usually used for `FAIL_POINT_INJECT_F` case fail_point::Return: return "Return"; - // `action` contain `print()`, which would only just print `action` string value and ignore the - // `function` passed case fail_point::Print: return "Print"; - // `action` contain `void()`, which would return args and execute the void type `function` - // passed, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to avoid `return` function case fail_point::Void: return "Void"; default: diff --git a/src/utils/fail_point_impl.h b/src/utils/fail_point_impl.h index 3e8d648d8a..685188c1fc 100644 --- a/src/utils/fail_point_impl.h +++ b/src/utils/fail_point_impl.h @@ -44,9 +44,20 @@ struct fail_point { enum task_type { + // `action` contain `off()`, which would `close` the fail_point whose `function` passed will + // not + // be executed; Off, + // `action` contain `return()`, which would `return` args passed and execute `return` type + // function passed. it's usually used for `FAIL_POINT_INJECT_F` Return, + // `action` contain `print()`, which would only just print `action` string value and ignore + // the + // `function` passed Print, + // `action` contain `void()`, which would return args and execute `function` passed that + // better mark as void `type`, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to + // avoid `return` function Void, }; From 0cda37dbd35c60656fd2e26ab95f9a79a0227b35 Mon Sep 17 00:00:00 2001 From: jiashuo Date: Wed, 9 Feb 2022 10:22:32 +0800 Subject: [PATCH 6/7] init --- src/utils/fail_point_impl.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/utils/fail_point_impl.h b/src/utils/fail_point_impl.h index 685188c1fc..a0ed0cb07e 100644 --- a/src/utils/fail_point_impl.h +++ b/src/utils/fail_point_impl.h @@ -45,15 +45,13 @@ struct fail_point enum task_type { // `action` contain `off()`, which would `close` the fail_point whose `function` passed will - // not - // be executed; + // not be executed; Off, // `action` contain `return()`, which would `return` args passed and execute `return` type // function passed. it's usually used for `FAIL_POINT_INJECT_F` Return, // `action` contain `print()`, which would only just print `action` string value and ignore - // the - // `function` passed + // the `function` passed Print, // `action` contain `void()`, which would return args and execute `function` passed that // better mark as void `type`, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to From dd405ec1a637c7f16578b73ebab7a52428926547 Mon Sep 17 00:00:00 2001 From: jiashuo Date: Wed, 9 Feb 2022 10:25:02 +0800 Subject: [PATCH 7/7] init --- src/utils/fail_point_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/fail_point_impl.h b/src/utils/fail_point_impl.h index a0ed0cb07e..2d82716d52 100644 --- a/src/utils/fail_point_impl.h +++ b/src/utils/fail_point_impl.h @@ -54,7 +54,7 @@ struct fail_point // the `function` passed Print, // `action` contain `void()`, which would return args and execute `function` passed that - // better mark as void `type`, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to + // better mark as `void` type, it's usually used for `FAIL_POINT_INJECT_NOT_RETURN_F` to // avoid `return` function Void, };