tests: Ignore notification type mismatches in `test_notification_ust_buffer_usage`

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 <kstewart@efficios.com>
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/ctl/format.hpp b/src/common/ctl/format.hpp
index 60b16a869d3ed45a021b08ad740966f4516fdbd3..85dea1bb2d8db2ef4365e1649e48dccda2ab4b07 100644 (file)
--- a/src/common/ctl/format.hpp
+++ b/src/common/ctl/format.hpp
@@ -172,6 +172,44 @@ struct formatter<lttng_event_rule> : formatter<std::string> {
        }
 };
 
+template <>
+struct formatter<lttng_condition_type> : formatter<std::string> {
+       template <typename FormatContextType>
+       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 f20cc9a08d44c5b09223a6124aa06d62d7702fc1..398e3420e38a2dd1d4d1637a92c039568a03aa81 100644 (file)
--- 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<lttng_kernel_probe_location,
                                                            lttng_kernel_probe_location_destroy>>;
 
+using notification_uptr = std::unique_ptr<
+       lttng_notification,
+       lttng::memory::create_deleter_class<lttng_notification, lttng_notification_destroy>>;
+
 } /* 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 edb3f27c274d49649eea5bcbd02a959de1081fec..8e72c3546b76d80344cdea968a06bb93c16155d0 100644 (file)
--- 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 <jonathan.rajotte-julien@efficios.com>
 
 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 2bb385f77cbcca8661851f359cf35508c79cada8..428276ad4d046201b8d8b4c8c104c23cf4c0fc4b 100644 (file)
--- a/tests/regression/tools/notification/notification.cpp
+++ b/tests/regression/tools/notification/notification.cpp
@@ -10,6 +10,9 @@
  */
 
 #include <common/compat/errno.hpp>
+#include <common/ctl/format.hpp>
+#include <common/ctl/memory.hpp>
+#include <common/format.hpp>
 #include <common/macros.hpp>
 
 #include <lttng/lttng.h>
@@ -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,
-                                                                            &notification);
-       } 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,
-                                                                            &notification);
-       } 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,
-                                                                            &notification);
-       } 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,
-                                                                            &notification);
-       } 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,
-                                                                            &notification);
-       } 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();