From 897eb8fc1fd853bbd4edf7a93116a871e959b61d Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 13 Feb 2025 14:54:35 -0500 Subject: [PATCH] tests: Ignore notification type mismatches in `test_notification_ust_buffer_usage` MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed Issue ============== Frequent aborts in the CI on this test. Cause ===== The notification queue can be racy with regards the clients and consumers being stopped and started as they are in this test. The conditions for notifications are evaluated during each iteration of the monitor thread, by default every ~ 1000000 us. This means that in the moments between commands more than one notification can be registered as the states of the buffers are assessed. Solution ======== Ignore notification with types that don't match the expected notification type. Known drawbacks =============== None. Change-Id: I62c269a218a473b53dc11809b99b7c45b0a9887a Tested-by: Kienan Stewart Signed-off-by: Kienan Stewart Signed-off-by: Jérémie Galarneau --- src/common/ctl/format.hpp | 38 ++++++ src/common/ctl/memory.hpp | 4 + .../regression/tools/notification/Makefile.am | 4 +- .../tools/notification/notification.cpp | 126 +++++++++++------- 4 files changed, 126 insertions(+), 46 deletions(-) diff --git a/src/common/ctl/format.hpp b/src/common/ctl/format.hpp index 60b16a869..85dea1bb2 100644 --- a/src/common/ctl/format.hpp +++ b/src/common/ctl/format.hpp @@ -172,6 +172,44 @@ struct formatter : formatter { } }; +template <> +struct formatter : formatter { + template + typename FormatContextType::iterator format(lttng_condition_type condition_type, + FormatContextType& ctx) const + { + const char *condition_type_name; + + switch (condition_type) { + case LTTNG_CONDITION_TYPE_UNKNOWN: + condition_type_name = "UNKNOWN"; + break; + case LTTNG_CONDITION_TYPE_SESSION_CONSUMED_SIZE: + condition_type_name = "SESSION_CONSUMED_SIZE"; + break; + case LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH: + condition_type_name = "BUFFER_USAGE_HIGH"; + break; + case LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW: + condition_type_name = "BUFFER_USAGE_LOW"; + break; + case LTTNG_CONDITION_TYPE_SESSION_ROTATION_ONGOING: + condition_type_name = "SESSION_ROTATION_ONGOING"; + break; + case LTTNG_CONDITION_TYPE_SESSION_ROTATION_COMPLETED: + condition_type_name = "SESSION_ROTATION_COMPLETED"; + break; + case LTTNG_CONDITION_TYPE_EVENT_RULE_MATCHES: + condition_type_name = "EVENT_RULE_MATCHES"; + break; + default: + std::abort(); + } + + return format_to(ctx.out(), "{{type={}}}", condition_type_name); + } +}; + } /* namespace fmt */ #endif /* LTTNG_COMMON_CTL_FORMAT_H */ diff --git a/src/common/ctl/memory.hpp b/src/common/ctl/memory.hpp index f20cc9a08..398e3420e 100644 --- a/src/common/ctl/memory.hpp +++ b/src/common/ctl/memory.hpp @@ -26,6 +26,10 @@ using kernel_location_uptr = lttng::memory::create_deleter_class>; +using notification_uptr = std::unique_ptr< + lttng_notification, + lttng::memory::create_deleter_class>; + } /* namespace lttng */ #endif /* LTTNG_COMMON_CTL_MEMORY_HPP */ diff --git a/tests/regression/tools/notification/Makefile.am b/tests/regression/tools/notification/Makefile.am index edb3f27c2..8e72c3546 100644 --- a/tests/regression/tools/notification/Makefile.am +++ b/tests/regression/tools/notification/Makefile.am @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only +# SPDX-FileCopyrightText: 2017 Jonathan Rajotte AM_CPPFLAGS += -I$(top_srcdir)/tests/utils @@ -57,7 +58,8 @@ base_client_LDADD = $(LIB_LTTNG_CTL) notification_SOURCES = notification.cpp # Tests the deprecated lttng_register_trigger() interface notification_CXXFLAGS = -Wno-deprecated-declarations $(AM_CXXFLAGS) -notification_LDADD = $(LIB_LTTNG_CTL) $(LIBTAP) -lm +notification_LDADD = $(LIB_LTTNG_CTL) $(LIBTAP) -lm \ + $(top_builddir)/src/vendor/fmt/libfmt.la rotation_SOURCES = rotation.c rotation_LDADD = $(LIB_LTTNG_CTL) $(LIBTAP) -lm diff --git a/tests/regression/tools/notification/notification.cpp b/tests/regression/tools/notification/notification.cpp index 2bb385f77..428276ad4 100644 --- a/tests/regression/tools/notification/notification.cpp +++ b/tests/regression/tools/notification/notification.cpp @@ -10,6 +10,9 @@ */ #include +#include +#include +#include #include #include @@ -1266,6 +1269,45 @@ end: lttng_condition_destroy(condition); } +struct notification_reception_result { + lttng_notification_channel_status status; + lttng::notification_uptr notification; +}; + +lttng::notification_uptr +get_next_notification_skip_type(lttng_notification_channel *notification_channel, + const lttng_condition_type ignored_condition_type) +{ + while (true) { + auto result = [notification_channel]() { + lttng_notification *raw_notification; + const auto status = lttng_notification_channel_get_next_notification( + notification_channel, &raw_notification); + + return notification_reception_result{ + status, lttng::notification_uptr(raw_notification) + }; + }(); + + switch (result.status) { + case LTTNG_NOTIFICATION_CHANNEL_STATUS_OK: + if (lttng_condition_get_type(lttng_notification_get_condition( + result.notification.get())) == ignored_condition_type) { + continue; + } else { + return std::move(result.notification); + } + + continue; + case LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED: + continue; + default: + /* An error has occurred. */ + return nullptr; + } + } +} + void test_buffer_usage_notification_channel(const char *session_name, const char *channel_name, const enum lttng_domain_type domain_type, @@ -1273,17 +1315,14 @@ void test_buffer_usage_notification_channel(const char *session_name, { int ret = 0; enum lttng_notification_channel_status nc_status; - struct lttng_action *low_action = nullptr; struct lttng_action *high_action = nullptr; - struct lttng_notification *notification = nullptr; struct lttng_notification_channel *notification_channel = nullptr; struct lttng_trigger *low_trigger = nullptr; struct lttng_trigger *high_trigger = nullptr; - struct lttng_condition *low_condition = nullptr; struct lttng_condition *high_condition = nullptr; - + lttng::notification_uptr notification; const double low_ratio = 0.0; const double high_ratio = 0.90; @@ -1335,17 +1374,18 @@ void test_buffer_usage_notification_channel(const char *session_name, stop_consumer(argv); lttng_start_tracing(session_name); - /* Wait for high notification */ - do { - nc_status = lttng_notification_channel_get_next_notification(notification_channel, - ¬ification); - } while (nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED); - ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK && notification && - lttng_condition_get_type(lttng_notification_get_condition(notification)) == + /* + * Wait for high notification. Skip any low-usage notification since the buffers may + * be observed under the low-usage threshold before being observed as being + * almost-full. + */ + notification = get_next_notification_skip_type(notification_channel, + LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW); + + ok(notification && + lttng_condition_get_type(lttng_notification_get_condition(notification.get())) == LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH, "High notification received after intermediary communication"); - lttng_notification_destroy(notification); - notification = nullptr; suspend_application(); lttng_stop_tracing_no_wait(session_name); @@ -1355,6 +1395,14 @@ void test_buffer_usage_notification_channel(const char *session_name, /* * Test that communication still work even if there is notification * waiting for consumption. + * + * This is racy in the sense that nothing guarantees that the sessiond will have + * sent a low-usage notification between wait_data_pending() and the call to + * unsubscribe. + * + * The goal of this test is to ensure that the communication channel is not blocked + * by a pending notification when sending a command and awaiting a reply (status + * code). In that sense, the test doesn't always exercise that specific code path. */ nc_status = lttng_notification_channel_unsubscribe(notification_channel, low_condition); @@ -1365,64 +1413,52 @@ void test_buffer_usage_notification_channel(const char *session_name, ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK, "Subscribe with pending notification"); - do { - nc_status = lttng_notification_channel_get_next_notification(notification_channel, - ¬ification); - } while (nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED); - ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK && notification && - lttng_condition_get_type(lttng_notification_get_condition(notification)) == + notification = get_next_notification_skip_type(notification_channel, + LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH); + + ok(notification && + lttng_condition_get_type(lttng_notification_get_condition(notification.get())) == LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW, "Low notification received after intermediary communication"); - lttng_notification_destroy(notification); - notification = nullptr; /* Stop consumer to force a high notification */ stop_consumer(argv); resume_application(); lttng_start_tracing(session_name); - do { - nc_status = lttng_notification_channel_get_next_notification(notification_channel, - ¬ification); - } while (nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED); - ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK && notification && - lttng_condition_get_type(lttng_notification_get_condition(notification)) == + notification = get_next_notification_skip_type(notification_channel, + LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW); + + ok(notification && + lttng_condition_get_type(lttng_notification_get_condition(notification.get())) == LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH, "High notification received after intermediary communication"); - lttng_notification_destroy(notification); - notification = nullptr; suspend_application(); lttng_stop_tracing_no_wait(session_name); resume_consumer(argv); wait_data_pending(session_name); - do { - nc_status = lttng_notification_channel_get_next_notification(notification_channel, - ¬ification); - } while (nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED); - ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK && notification && - lttng_condition_get_type(lttng_notification_get_condition(notification)) == + notification = get_next_notification_skip_type(notification_channel, + LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH); + + ok(notification && + lttng_condition_get_type(lttng_notification_get_condition(notification.get())) == LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW, "Low notification received after re-subscription"); - lttng_notification_destroy(notification); - notification = nullptr; stop_consumer(argv); resume_application(); /* Stop consumer to force a high notification */ lttng_start_tracing(session_name); - do { - nc_status = lttng_notification_channel_get_next_notification(notification_channel, - ¬ification); - } while (nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_INTERRUPTED); - ok(nc_status == LTTNG_NOTIFICATION_CHANNEL_STATUS_OK && notification && - lttng_condition_get_type(lttng_notification_get_condition(notification)) == + notification = get_next_notification_skip_type(notification_channel, + LTTNG_CONDITION_TYPE_BUFFER_USAGE_LOW); + + ok(notification && + lttng_condition_get_type(lttng_notification_get_condition(notification.get())) == LTTNG_CONDITION_TYPE_BUFFER_USAGE_HIGH, "High notification"); - lttng_notification_destroy(notification); - notification = nullptr; suspend_application(); -- 2.39.5