Fix: relayd: rotation failure for multi-domain session
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Wed, 12 Jan 2022 20:48:00 +0000 (15:48 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Tue, 25 Jan 2022 22:35:53 +0000 (17:35 -0500)
Observed issue
==============

Rotating a multi-domain streaming session results in the following
error:

  $ lttng rotate
  Waiting for rotation to complete...
  Error: Failed to retrieve rotation state.

Meanwhile, the relay daemon logs indicate the following:

  DBG1 - 14:56:04.213163667 [265774/265778]: lttng_trace_chunk_rename_path from .tmp_new_chunk to (null) (in lttng_trace_chunk_rename_path_no_lock() at trace-chunk.cpp:759)
  PERROR - 14:56:04.213242941 [265774/265778]: Failed to move trace chunk directory ".tmp_new_chunk" to "20220112T145604-0500-1": No such file or directory (in lttng_trace_chunk_rename_path_no_lock() at trace-chunk.cpp:799)
  DBG1 - 14:56:04.213396931 [265774/265778]: aborting session 2 (in session_abort() at session.cpp:588)
  DBG1 - 14:56:04.213512198 [265774/265778]: Control connection closed with 22 (in relay_thread_close_connection() at main.cpp:3874)

The 'abort' of session 2 here causes the kernel consumer to fail to
consume subbuffers:

  Error: Relayd send index failed. Cleaning up relayd 3.
  Error: Error consuming subbuffer: (0)
  [...]

Cause
=====

Following the flow of execution in the relay daemon shows that different
trace chunks are used by the two relay sessions that result from the
streaming of a single multi-domain session. Both trace chunks "own" the
same output directory.

When a rotation is performed, the first trace chunk to be closed will
move the directory. Then, the second trace chunk to be closed will
attempt to do the same, failing to do so as seen in the relay daemon
log.

Solution
========

Using different trace chunk instances for relay sessions belonging to a
single sessiond session goes against the intended use of the sessiond
trace chunk registry.

A sessiond trace chunk registry allows the relay daemon to share trace
chunks used by different "relay sessions" when they were created for the
same user-visible session daemon session. Tracing multiple domains (e.g.
ust and kernel) results in per-domain relay sessions being created.

Sharing trace chunks, and their output directory more specifically, is
essential to properly implement session rotations. The sharing of output
directory handles allows directory renames to be performed once and
without races that would stem from from multiple renames.

The reason why sessiond trace chunk registry returns different trace
chunk instances for two relay sessions is that the wrong session `id` is
used to publish trace chunks. The `id` that must be used to share trace
chunks accross the relay sessions that belong to the same sessiond
session is `id_sessiond`.

`id_sessiond` is optional as it is only provided by consumers v2.11+.
Otherwise, it is fine to use the relay session `id`: it is a unique id
for a given session daemon instance and those consumers will not issue a
session rotation (or clear) as the feature didn't exist.

A reference counting bug revealed by this change is also fixed in the
implementation of the sessiond trace chunk registry.

When the trace chunk is first published, two references to the published
chunks exist. One is taken by the registry while the other is being
returned to the caller. In the use case of the relay daemon, the
reference held by the registry itself is undesirable.

We want the trace chunk to be removed from the registry as soon as it is
not being used by the relay daemon (through a session or a stream). This
differs from the behaviour of the consumer daemon which relies on an
explicit command from the session daemon to release the registry's
reference.

In cases where the trace chunk had already been published, the reference
belonging to the sessiond trace chunk registry instance has already been
'put' by the firt publication. We must simply return the published trace
chunk with a reference taken on behalf of the caller.

Known drawbacks
===============

None.

Change-Id: Ic33443b114a87574a1b26ac5ccb022e47f886ddd
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
src/bin/lttng-relayd/main.c
src/bin/lttng-relayd/sessiond-trace-chunks.c
src/common/trace-chunk-registry.h
src/common/trace-chunk.c

index bafb225f1e4e5314e9eb9fa503ced0fdf1ceb6a6..7ffc1e9c5824cce0bcc8ac2480bcd95c4bd5dc60 100644 (file)
@@ -2653,7 +2653,10 @@ static int relay_rotate_session_streams(
                 */
                next_trace_chunk = sessiond_trace_chunk_registry_get_chunk(
                                sessiond_trace_chunk_registry,
-                               session->sessiond_uuid, session->id,
+                               session->sessiond_uuid,
+                               conn->session->id_sessiond.is_set ?
+                                       conn->session->id_sessiond.value :
+                                       conn->session->id,
                                rotate_streams.new_chunk_id.value);
                if (!next_trace_chunk) {
                        char uuid_str[LTTNG_UUID_STR_LEN];
@@ -2868,7 +2871,9 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr,
        published_chunk = sessiond_trace_chunk_registry_publish_chunk(
                        sessiond_trace_chunk_registry,
                        conn->session->sessiond_uuid,
-                       conn->session->id,
+                       conn->session->id_sessiond.is_set ?
+                               conn->session->id_sessiond.value :
+                               conn->session->id,
                        chunk);
        if (!published_chunk) {
                char uuid_str[LTTNG_UUID_STR_LEN];
@@ -2976,7 +2981,9 @@ static int relay_close_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr,
        chunk = sessiond_trace_chunk_registry_get_chunk(
                        sessiond_trace_chunk_registry,
                        conn->session->sessiond_uuid,
-                       conn->session->id,
+                       conn->session->id_sessiond.is_set ?
+                               conn->session->id_sessiond.value :
+                               conn->session->id,
                        chunk_id);
        if (!chunk) {
                char uuid_str[LTTNG_UUID_STR_LEN];
index 3db2133ed87f1fc0263834995cbb3ecea7737545..8ce71905959488765d5e1a0234427421415d35be 100644 (file)
@@ -360,6 +360,7 @@ struct lttng_trace_chunk *sessiond_trace_chunk_registry_publish_chunk(
        char uuid_str[LTTNG_UUID_STR_LEN];
        char chunk_id_str[MAX_INT_DEC_LEN(typeof(chunk_id))] = "-1";
        struct lttng_trace_chunk *published_chunk = NULL;
+       bool trace_chunk_already_published;
 
        lttng_uuid_to_str(sessiond_uuid, uuid_str);
        lttng_uuid_copy(key.sessiond_uuid, sessiond_uuid);
@@ -393,21 +394,31 @@ struct lttng_trace_chunk *sessiond_trace_chunk_registry_publish_chunk(
                goto end;
        }
 
-        published_chunk = lttng_trace_chunk_registry_publish_chunk(
-                       element->trace_chunk_registry, session_id, new_chunk);
+       published_chunk = lttng_trace_chunk_registry_publish_chunk_published(
+                       element->trace_chunk_registry, session_id, new_chunk,
+                       &trace_chunk_already_published);
+
        /*
-        * At this point, two references to the published chunks exist. One
-        * is taken by the registry while the other is being returned to the
-        * caller. In the use case of the relay daemon, the reference held
-        * by the registry itself is undesirable.
+        * When the trace chunk is first published, two references to the
+        * published chunks exist. One is taken by the registry while the other
+        * is being returned to the caller. In the use case of the relay daemon,
+        * the reference held by the registry itself is undesirable.
         *
         * We want the trace chunk to be removed from the registry as soon
         * as it is not being used by the relay daemon (through a session
         * or a stream). This differs from the behaviour of the consumer
         * daemon which relies on an explicit command from the session
         * daemon to release the registry's reference.
+        *
+        * In cases where the trace chunk had already been published,
+        * the reference belonging to the sessiond trace chunk
+        * registry instance has already been 'put'. We simply return
+        * the published trace chunk with a reference taken on behalf of the
+        * caller.
         */
-       lttng_trace_chunk_put(published_chunk);
+       if (!trace_chunk_already_published) {
+               lttng_trace_chunk_put(published_chunk);
+       }
 end:
        trace_chunk_registry_ht_element_put(element);
        return published_chunk;
index 00dd7fcf0343ebd8a38aa50f8189fdb91bdb55eb..b7905cac95d55a6d3b12f8b17c14649e44c35aa6 100644 (file)
@@ -61,6 +61,30 @@ struct lttng_trace_chunk *lttng_trace_chunk_registry_publish_chunk(
                struct lttng_trace_chunk_registry *registry,
                uint64_t session_id, struct lttng_trace_chunk *chunk);
 
+/*
+ * Adds the `previously_published` parameter which allows the caller
+ * to know if a trace chunk equivalent to `chunk` was previously published.
+ * 
+ * The registry holds a reference to the published trace chunks it contains.
+ * Trace chunks automatically unpublish themselves from their registry on
+ * destruction.
+ *
+ * This information is necessary to drop the reference of newly published
+ * chunks when a user doesn't wish to explicitly maintain all references
+ * to a given trace chunk.
+ * 
+ * For instance, the relay daemon doesn't need the registry to hold a
+ * reference since it controls the lifetime of its trace chunks.
+ * Conversely, the consumer daemons rely on the session daemon to inform
+ * them of the end of life of a trace chunk and the trace chunks don't
+ * belong to a specific top-level object: they are always retrieved from
+ * the registry by `id`.
+ */
+struct lttng_trace_chunk *lttng_trace_chunk_registry_publish_chunk_published(
+               struct lttng_trace_chunk_registry *registry,
+               uint64_t session_id, struct lttng_trace_chunk *chunk,
+               bool *previously_published);
+
 /*
  * Look-up a trace chunk by session_id and chunk_id.
  * A reference is acquired on behalf of the caller.
index a18b2077e5dc9c35680d96507c7491f515824471..2f31fb689ba0a683e074c74268017b6c8a659a12 100644 (file)
@@ -2001,7 +2001,20 @@ LTTNG_HIDDEN
 struct lttng_trace_chunk *
 lttng_trace_chunk_registry_publish_chunk(
                struct lttng_trace_chunk_registry *registry,
-               uint64_t session_id, struct lttng_trace_chunk *chunk)
+               uint64_t session_id,
+               struct lttng_trace_chunk *chunk)
+{
+       bool unused;
+
+       return lttng_trace_chunk_registry_publish_chunk_published(
+                       registry, session_id, chunk, &unused);
+}
+
+struct lttng_trace_chunk *
+lttng_trace_chunk_registry_publish_chunk_published(
+               struct lttng_trace_chunk_registry *registry,
+               uint64_t session_id, struct lttng_trace_chunk *chunk,
+               bool *previously_published)
 {
        struct lttng_trace_chunk_registry_element *element;
        unsigned long element_hash;
@@ -2036,6 +2049,7 @@ lttng_trace_chunk_registry_publish_chunk(
                        element->registry = registry;
                        /* Acquire a reference for the caller. */
                        if (lttng_trace_chunk_get(&element->chunk)) {
+                               *previously_published = false;
                                break;
                        } else {
                                /*
@@ -2062,6 +2076,7 @@ lttng_trace_chunk_registry_publish_chunk(
                if (lttng_trace_chunk_get(published_chunk)) {
                        lttng_trace_chunk_put(&element->chunk);
                        element = published_element;
+                       *previously_published = true;
                        break;
                }
                /*
This page took 0.030595 seconds and 5 git commands to generate.