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>
Thu, 13 Jan 2022 17:12:22 +0000 (12:12 -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.cpp
src/bin/lttng-relayd/sessiond-trace-chunks.cpp
src/common/trace-chunk-registry.h
src/common/trace-chunk.cpp

index d3f8b3c9e30234a98eed30f49831028a3f9a99bf..942bfa4d5ab3abded6b6031e0498042e5e1dfbb9 100644 (file)
@@ -2663,7 +2663,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];
@@ -2885,7 +2888,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];
@@ -2991,7 +2996,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 e0c46e34ad4a7436e344b178a1302bd9fd06c64b..18ef586d00c85d62e5b92bfbc083979dc32e5bf6 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);
@@ -394,20 +395,29 @@ struct lttng_trace_chunk *sessiond_trace_chunk_registry_publish_chunk(
        }
 
        published_chunk = lttng_trace_chunk_registry_publish_chunk(
-                       element->trace_chunk_registry, session_id, new_chunk);
+                       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 0eee0c21c4e918ce14d2c6ebccc8f21cf25c9379..5442d7997893eb744121d7661706d677fcbae127 100644 (file)
@@ -57,6 +57,29 @@ void lttng_trace_chunk_registry_destroy(
 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(
+               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.
index eed3d558b0181d269501fb485e7ec63298a8f8d5..ad7a983fbc78e50ca78db0c038aeae2f4244627e 100644 (file)
@@ -1984,7 +1984,20 @@ end:
 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(
+                       registry, session_id, chunk, &unused);
+}
+
+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,
+               bool *previously_published)
 {
        struct lttng_trace_chunk_registry_element *element;
        unsigned long element_hash;
@@ -2019,6 +2032,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 {
                                /*
@@ -2045,6 +2059,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.03111 seconds and 4 git commands to generate.