Fix: relayd: per-pid live: no new metadata vs close

When using lttng-live on a per-pid UST trace, the metadata stream is
closed after an application exits. The live viewer observes that the
stream has no more data when getting the metadata returns 0 bytes.

Internally in the relay daemon, it is assumed that a metadata stream
can be put (and thus removed) as soon as all the metadata has been sent
to the viewer, but this is not quite right. The viewer actually needs to
observe a 0-byte reply after receiving all the metadata in order to
gracefully perceive the metadata stream hang up.

Therefore, add a no_new_metadata_notified flag to the metadata stream
to track whether that 0-byte message has been sent to the viewer since
the last metadata content was sent, and postpone the reclamation of the
metadata stream until it is closed _and_ that 0-byte reply was sent to
the live viewer.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I0a05332284299d62b832046e4f9d22b6029c3a3e
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
index f00f07db1b8e63fc280c5a868379da2a47a68ade..1b25671d5f76a82ad83a33ceceb3691e8794457c 100644 (file)
--- a/src/bin/lttng-relayd/live.c
+++ b/src/bin/lttng-relayd/live.c
@@ -1724,7 +1724,23 @@ int viewer_get_metadata(struct relay_connection *conn)
 
        len = vstream->stream->metadata_received - vstream->metadata_sent;
        if (len == 0) {
+               /*
+                * The live viewers expect to receive a NO_NEW_METADATA
+                * status before a stream disappears, otherwise they abort the
+                * entire live connection when receiving an error status.
+                */
                reply.status = htobe32(LTTNG_VIEWER_NO_NEW_METADATA);
+               /*
+                * The live viewer considers a closed 0 byte metadata stream as
+                * an error.
+                */
+               if (vstream->metadata_sent > 0) {
+                       vstream->stream->no_new_metadata_notified = true;
+                       if (vstream->stream->closed) {
+                               /* Release ownership for the viewer metadata stream. */
+                               viewer_stream_put(vstream);
+                       }
+               }
                goto send_reply;
        }
 
@@ -1772,12 +1788,6 @@ int viewer_get_metadata(struct relay_connection *conn)
                goto error;
        }
        vstream->metadata_sent += read_len;
-       if (vstream->metadata_sent == vstream->stream->metadata_received
-                       && vstream->stream->closed) {
-               /* Release ownership for the viewer metadata stream. */
-               viewer_stream_put(vstream);
-       }
-
        reply.status = htobe32(LTTNG_VIEWER_METADATA_OK);
 
        goto send_reply;

diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index 9581d03eb074e1351ac0774034fb31a205c75a9b..54f31d7218a175fa16cbc8a7827f53c393a7f616 100644 (file)
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -1521,7 +1521,7 @@ static int relay_close_stream(const struct lttcomm_relayd_hdr *recv_hdr,
 
                vstream = viewer_stream_get_by_id(stream->stream_handle);
                if (vstream) {
-                       if (vstream->metadata_sent == stream->metadata_received) {
+                       if (stream->no_new_metadata_notified) {
                                /*
                                 * Since all the metadata has been sent to the
                                 * viewer and that we have a request to close

diff --git a/src/bin/lttng-relayd/stream.c b/src/bin/lttng-relayd/stream.c
index c0aeb17190d187e12155e8a162ef89204647df2b..427d30a00602c7448223e2c58b27f4eab9b9bf58 100644 (file)
--- a/src/bin/lttng-relayd/stream.c
+++ b/src/bin/lttng-relayd/stream.c
@@ -1086,8 +1086,14 @@ int stream_write(struct relay_stream *stream,
        }
 
        if (stream->is_metadata) {
-               stream->metadata_received += packet ? packet->size : 0;
-               stream->metadata_received += padding_len;
+               size_t recv_len;
+
+               recv_len = packet ? packet->size : 0;
+               recv_len += padding_len;
+               stream->metadata_received += recv_len;
+               if (recv_len) {
+                       stream->no_new_metadata_notified = false;
+               }
        }
 
        DBG("Wrote to %sstream %" PRIu64 ": data_length = %zu, padding_length = %zu",

diff --git a/src/bin/lttng-relayd/stream.h b/src/bin/lttng-relayd/stream.h
index 02948fa8e4c3439906dca9df5710c0d5f992f644..c88e725c012955473de8dc50df6d707b9ff35537 100644 (file)
--- a/src/bin/lttng-relayd/stream.h
+++ b/src/bin/lttng-relayd/stream.h
@@ -175,6 +175,8 @@ struct relay_stream {
        struct cds_list_head recv_node;
        /* Protected by session lock. */
        bool published;
+       /* Notified viewer that no new metadata is available. */
+       bool no_new_metadata_notified;
        /*
         * Node of stream within global stream hash table.
         */