Fix: relayd: session destruction does not complete in live mode
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 19 Sep 2019 17:48:20 +0000 (13:48 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 19 Sep 2019 18:33:21 +0000 (14:33 -0400)
Live clients currently hold a reference to their relay_session's trace
chunk through the viewer streams. This causes the destruction of live
session to hang for as long as a client is consuming a session's
contents.

The reason for the hang itself is that the "quiet" rotation performed
as part of the session's destruction can never complete. Indeed, a
rotation completes when the last reference to the trace chunk being
archived is released. Since viewer streams hold references to this
trace chunk, they bound its lifetime to their own lifetime, resulting
in the destruction operation appearing to hang to users of
liblttng-ctl / the lttng client.

Since session rotations are not allowed for live sessions, it is safe
to presume that a session's output folder hierarchy will not change
over its lifetime with the exception of sub-folder additions, such as
in per-PID tracing mode where new traces may be added at any time
within the session's output directory.

The viewer session now copies its corresponding relay_session's trace
chunk to obtain a 'user' mode trace chunk. A user mode trace chunk
will prevent the viewer session from modifying the session output
hierarchy and prevent it from keeping the relay_session's trace chunk
"alive". The viewer stream simply use this trace chunk to perform
their filesystem operations, but they no longer extend the lifetime of
the relayd_session trace chunks.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
src/bin/lttng-relayd/live.c
src/bin/lttng-relayd/viewer-session.c
src/bin/lttng-relayd/viewer-session.h
src/bin/lttng-relayd/viewer-stream.c
src/bin/lttng-relayd/viewer-stream.h

index dc8238c9499b876de333a7c313e1e95135e847aa..88bb7363db962a41e65c8ccff4a6a654cbc666a3 100644 (file)
@@ -276,10 +276,13 @@ end_unlock:
  *
  * Return 0 on success or else a negative value.
  */
-static
-int make_viewer_streams(struct relay_session *session,
-               enum lttng_viewer_seek seek_t, uint32_t *nb_total, uint32_t *nb_unsent,
-               uint32_t *nb_created, bool *closed)
+static int make_viewer_streams(struct relay_session *session,
+               struct lttng_trace_chunk *viewer_trace_chunk,
+               enum lttng_viewer_seek seek_t,
+               uint32_t *nb_total,
+               uint32_t *nb_unsent,
+               uint32_t *nb_created,
+               bool *closed)
 {
        int ret;
        struct lttng_ht_iter iter;
@@ -321,7 +324,8 @@ int make_viewer_streams(struct relay_session *session,
                        }
                        vstream = viewer_stream_get_by_id(stream->stream_handle);
                        if (!vstream) {
-                               vstream = viewer_stream_create(stream, seek_t);
+                               vstream = viewer_stream_create(stream,
+                                               viewer_trace_chunk, seek_t);
                                if (!vstream) {
                                        ret = -1;
                                        ctf_trace_put(ctf_trace);
@@ -906,7 +910,7 @@ int viewer_get_new_streams(struct relay_connection *conn)
        uint32_t nb_created = 0, nb_unsent = 0, nb_streams = 0, nb_total = 0;
        struct lttng_viewer_new_streams_request request;
        struct lttng_viewer_new_streams_response response;
-       struct relay_session *session;
+       struct relay_session *session = NULL;
        uint64_t session_id;
        bool closed = false;
 
@@ -944,12 +948,23 @@ int viewer_get_new_streams(struct relay_connection *conn)
        response.status = htobe32(LTTNG_VIEWER_NEW_STREAMS_OK);
 
        pthread_mutex_lock(&session->lock);
-       ret = make_viewer_streams(session, LTTNG_VIEWER_SEEK_LAST, &nb_total, &nb_unsent,
+       if (!conn->viewer_session->current_trace_chunk &&
+                       session->current_trace_chunk) {
+               ret = viewer_session_set_trace_chunk(conn->viewer_session,
+                               session->current_trace_chunk);
+               if (ret) {
+                       goto error_unlock_session;
+               }
+       }
+       ret = make_viewer_streams(session,
+                       conn->viewer_session->current_trace_chunk,
+                       LTTNG_VIEWER_SEEK_LAST, &nb_total, &nb_unsent,
                        &nb_created, &closed);
-       pthread_mutex_unlock(&session->lock);
        if (ret < 0) {
-               goto end_put_session;
+               goto error_unlock_session;
        }
+       pthread_mutex_unlock(&session->lock);
+
        /* Only send back the newly created streams with the unsent ones. */
        nb_streams = nb_created + nb_unsent;
        response.streams_count = htobe32(nb_streams);
@@ -998,6 +1013,10 @@ end_put_session:
        }
 error:
        return ret;
+error_unlock_session:
+       pthread_mutex_unlock(&session->lock);
+       session_put(session);
+       return ret;
 }
 
 /*
@@ -1073,12 +1092,20 @@ int viewer_attach_session(struct relay_connection *conn)
                goto send_reply;
        }
 
-       ret = make_viewer_streams(session, seek_type, &nb_streams, NULL,
-                       NULL, &closed);
+       if (!conn->viewer_session->current_trace_chunk &&
+                       session->current_trace_chunk) {
+               ret = viewer_session_set_trace_chunk(conn->viewer_session,
+                               session->current_trace_chunk);
+               if (ret) {
+                       goto end_put_session;
+               }
+       }
+       ret = make_viewer_streams(session,
+                       conn->viewer_session->current_trace_chunk, seek_type,
+                       &nb_streams, NULL, NULL, &closed);
        if (ret < 0) {
                goto end_put_session;
        }
-
        pthread_mutex_unlock(&session->lock);
        session_put(session);
        session = NULL;
@@ -1158,7 +1185,7 @@ static int try_open_index(struct relay_viewer_stream *vstream,
                goto end;
        }
        vstream->index_file = lttng_index_file_create_from_trace_chunk_read_only(
-                       rstream->trace_chunk, rstream->path_name,
+                       vstream->stream_file.trace_chunk, rstream->path_name,
                        rstream->channel_name, rstream->tracefile_size,
                        vstream->current_tracefile_id,
                        lttng_to_index_major(connection_major, connection_minor),
index 5f9c7c5769f799d5ff101446b730b06c340b5f4f..7ff58ed967842b75881a43c7de4fa5f029cc23fe 100644 (file)
@@ -101,6 +101,7 @@ static int viewer_session_detach(struct relay_viewer_session *vsession,
 
 void viewer_session_destroy(struct relay_viewer_session *vsession)
 {
+       lttng_trace_chunk_put(vsession->current_trace_chunk);
        free(vsession);
 }
 
@@ -183,3 +184,25 @@ end:
        pthread_mutex_unlock(&session->lock);
        return found;
 }
+
+int viewer_session_set_trace_chunk(struct relay_viewer_session *vsession,
+               struct lttng_trace_chunk *relay_session_trace_chunk)
+{
+       int ret = 0;
+       struct lttng_trace_chunk *viewer_chunk;
+
+       assert(relay_session_trace_chunk);
+       assert(!vsession->current_trace_chunk);
+
+       DBG("Copying relay session's current trace chunk to the viewer session");
+       viewer_chunk = lttng_trace_chunk_copy(relay_session_trace_chunk);
+       if (!viewer_chunk) {
+               ERR("Failed to create a viewer trace chunk from the relay session's current chunk");
+               ret = -1;
+               goto end;
+       }
+
+       vsession->current_trace_chunk = viewer_chunk;
+end:
+       return ret;
+}
index 249a483d5f16a05486f9a648f511971983c74729..c127f69c1b42b18cd3b7e1e277138d4aeb6b4b1d 100644 (file)
@@ -27,6 +27,7 @@
 #include <urcu/ref.h>
 
 #include <common/hashtable/hashtable.h>
+#include <common/trace-chunk.h>
 
 #include "session.h"
 
@@ -39,6 +40,8 @@ struct relay_viewer_session {
         */
        struct cds_list_head session_list;      /* RCU list. */
        pthread_mutex_t session_list_lock;      /* Protects list updates. */
+       /* Once set, the current trace chunk of a viewer must not change. */
+       struct lttng_trace_chunk *current_trace_chunk;
 };
 
 struct relay_viewer_session *viewer_session_create(void);
@@ -51,5 +54,7 @@ int viewer_session_is_attached(struct relay_viewer_session *vsession,
                struct relay_session *session);
 void viewer_session_close_one_session(struct relay_viewer_session *vsession,
                struct relay_session *session);
+int viewer_session_set_trace_chunk(struct relay_viewer_session *vsession,
+               struct lttng_trace_chunk *relay_session_trace_chunk);
 
 #endif /* _VIEWER_SESSION_H */
index 70cd1cea15c13c27823c7555383d37f26f07cae9..25ee35e5cadb97787a23e238dccfbcdae0924820 100644 (file)
@@ -42,11 +42,12 @@ static void viewer_stream_destroy_rcu(struct rcu_head *head)
 }
 
 struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream,
+               struct lttng_trace_chunk *viewer_trace_chunk,
                enum lttng_viewer_seek seek_t)
 {
        struct relay_viewer_stream *vstream = NULL;
        const bool acquired_reference = lttng_trace_chunk_get(
-                       stream->trace_chunk);
+                       viewer_trace_chunk);
 
        if (!acquired_reference) {
                goto error;
@@ -58,7 +59,8 @@ struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream,
                goto error;
        }
 
-       vstream->stream_file.trace_chunk = stream->trace_chunk;
+       vstream->stream_file.trace_chunk = viewer_trace_chunk;
+       viewer_trace_chunk = NULL;
        vstream->path_name = lttng_strndup(stream->path_name, LTTNG_VIEWER_PATH_MAX);
        if (vstream->path_name == NULL) {
                PERROR("relay viewer path_name alloc");
@@ -130,7 +132,8 @@ struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream,
                const uint32_t connection_minor = stream->trace->session->minor;
 
                vstream->index_file = lttng_index_file_create_from_trace_chunk_read_only(
-                               stream->trace_chunk, stream->path_name,
+                               vstream->stream_file.trace_chunk,
+                               stream->path_name,
                                stream->channel_name, stream->tracefile_size,
                                vstream->current_tracefile_id,
                                lttng_to_index_major(connection_major,
@@ -170,6 +173,9 @@ error:
        if (vstream) {
                viewer_stream_destroy(vstream);
        }
+       if (viewer_trace_chunk) {
+               lttng_trace_chunk_put(viewer_trace_chunk);
+       }
        return NULL;
 }
 
@@ -313,7 +319,8 @@ int viewer_stream_rotate(struct relay_viewer_stream *vstream)
        }
        vstream->index_file =
                        lttng_index_file_create_from_trace_chunk_read_only(
-                                       stream->trace_chunk, stream->path_name,
+                                       vstream->stream_file.trace_chunk,
+                                       stream->path_name,
                                        stream->channel_name,
                                        stream->tracefile_size,
                                        vstream->current_tracefile_id,
index 7c42dfad39a7444038400101b88334aae622237a..18a3521d7c117e049915a0ad2f791786493c66c0 100644 (file)
@@ -81,6 +81,7 @@ struct relay_viewer_stream {
 };
 
 struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream,
+               struct lttng_trace_chunk *viewer_trace_chunk,
                enum lttng_viewer_seek seek_t);
 
 struct relay_viewer_stream *viewer_stream_get_by_id(uint64_t id);
This page took 0.033059 seconds and 5 git commands to generate.