Fix: src.ctf.lttng-live: decode metadata even on _STATUS_CLOSED

When receiving a _GET_ONE_METADATA_STATUS_CLOSED from the
`lttng_live_get_one_metadata_packet()` function it means that the
metadata stream was closed by the relay and we will no longer get any
new metadata. But, it also means that everything we received so far is
valid and can be decoded.

So rather than returning _LIVE_ITERATOR_STATUS_END the right thing to do
is to go on with the decoding of the metadata packets.

Also did the following cleanups:
  * Merge the `lttng_live_metadata::trace` and
  `lttng_live_trace::new_metadata_needed` into a new tri-state enum
  `lttng_live_metadata_stream_state` in the `lttng_live_trace` struct.
  Those two fields were tightly linked and it's simpler to have a single
  field.

  * Remove the "poking of the trace metadata" using the
  `new_metadata_needed` as it's no longer necessary since all references
  are now borrowed.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I3e18365d5cfeaa77935409bdfe8fdda52fa5b636
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2351

diff --git a/src/plugins/ctf/lttng-live/data-stream.c b/src/plugins/ctf/lttng-live/data-stream.c
index 6414805351deb4157ce66280ee60be2993b3cbfb..79f6c3fa97cd540cbebfd1f80323c23836ef38df 100644 (file)
--- a/src/plugins/ctf/lttng-live/data-stream.c
+++ b/src/plugins/ctf/lttng-live/data-stream.c
@@ -317,10 +317,5 @@ void lttng_live_stream_iterator_destroy(
        /* Track the number of active stream iterator. */
        stream_iter->trace->session->lttng_live_msg_iter->active_stream_iter--;
 
-       /*
-        * Ensure we poke the trace metadata in the future, which is
-        * required to release the metadata reference on the trace.
-        */
-       stream_iter->trace->new_metadata_needed = true;
        g_free(stream_iter);
 }

diff --git a/src/plugins/ctf/lttng-live/lttng-live.c b/src/plugins/ctf/lttng-live/lttng-live.c
index 82ebbff9b01f91afe8a15b2fc88c4c307fcb0d96..078fe94d787289cdea05249946aa194f48172205 100644 (file)
--- a/src/plugins/ctf/lttng-live/lttng-live.c
+++ b/src/plugins/ctf/lttng-live/lttng-live.c
@@ -196,7 +196,7 @@ struct lttng_live_trace *lttng_live_create_trace(struct lttng_live_session *sess
        trace->stream_iterators = g_ptr_array_new_with_free_func(
                (GDestroyNotify) lttng_live_stream_iterator_destroy);
        BT_ASSERT(trace->stream_iterators);
-       trace->new_metadata_needed = true;
+       trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED;
        g_ptr_array_add(session->traces, trace);
 
        goto end;
@@ -395,7 +395,8 @@ enum lttng_live_iterator_status lttng_live_iterator_next_handle_one_no_data_stre
        enum lttng_live_stream_state orig_state = lttng_live_stream->state;
        struct packet_index index;
 
-       if (lttng_live_stream->trace->new_metadata_needed) {
+       if (lttng_live_stream->trace->metadata_stream_state ==
+                       LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED) {
                ret = LTTNG_LIVE_ITERATOR_STATUS_CONTINUE;
                goto end;
        }
@@ -543,7 +544,11 @@ void lttng_live_force_new_streams_and_metadata(struct lttng_live_msg_iter *lttng
                                trace_idx++) {
                        struct lttng_live_trace *trace =
                                g_ptr_array_index(session->traces, trace_idx);
-                       trace->new_metadata_needed = true;
+
+                       BT_ASSERT(trace->metadata_stream_state !=
+                               LTTNG_LIVE_METADATA_STREAM_STATE_CLOSED);
+
+                       trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED;
                }
        }
 }
@@ -810,7 +815,7 @@ enum lttng_live_iterator_status lttng_live_iterator_next_handle_one_active_data_
                                trace_idx++) {
                        struct lttng_live_trace *trace =
                                g_ptr_array_index(session->traces, trace_idx);
-                       if (trace->new_metadata_needed) {
+                       if (trace->metadata_stream_state == LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED) {
                                ret = LTTNG_LIVE_ITERATOR_STATUS_CONTINUE;
                                goto end;
                        }

diff --git a/src/plugins/ctf/lttng-live/lttng-live.h b/src/plugins/ctf/lttng-live/lttng-live.h
index 1447cc5d2815ea7bb16a65a6f934b36191b8d2c5..58131f8208640668134a9e9f2ea68fff0cbff817 100644 (file)
--- a/src/plugins/ctf/lttng-live/lttng-live.h
+++ b/src/plugins/ctf/lttng-live/lttng-live.h
@@ -129,14 +129,32 @@ struct lttng_live_metadata {
        bt_logging_level log_level;
        bt_self_component *self_comp;
 
-       /* Weak reference. */
-       struct lttng_live_trace *trace;
-
        uint64_t stream_id;
        /* Weak reference. */
        struct ctf_metadata_decoder *decoder;
 };
 
+enum lttng_live_metadata_stream_state {
+       /*
+        * The metadata needs to be updated. This is either because we just
+        * created the trace and haven't asked yet, or the relay specifically
+        * told us that new metadata is available.
+        */
+       LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED,
+       /*
+        * The metadata was updated and the relay has not told us we need to
+        * update it yet.
+        */
+       LTTNG_LIVE_METADATA_STREAM_STATE_NOT_NEEDED,
+       /*
+        * The relay has closed this metadata stream. We set this in reaction
+        * to a LTTNG_VIEWER_METADATA_ERR reply to a LTTNG_VIEWER_GET_METADATA
+        * command to the relay. If this field is set, we have received all the
+        * metadata that we are ever going to get for that metadata stream.
+        */
+       LTTNG_LIVE_METADATA_STREAM_STATE_CLOSED,
+};
+
 struct lttng_live_trace {
        bt_logging_level log_level;
        bt_self_component *self_comp;
@@ -161,7 +179,7 @@ struct lttng_live_trace {
        /* Owned by this. */
        GPtrArray *stream_iterators;
 
-       bool new_metadata_needed;
+       enum lttng_live_metadata_stream_state metadata_stream_state;
 };
 
 struct lttng_live_session {

diff --git a/src/plugins/ctf/lttng-live/metadata.c b/src/plugins/ctf/lttng-live/metadata.c
index d347534da7e006457c845df89e33bac3ad4b24cd..175b8071f0238098788d842b99ba3823e2a189b9 100644 (file)
--- a/src/plugins/ctf/lttng-live/metadata.c
+++ b/src/plugins/ctf/lttng-live/metadata.c
@@ -145,11 +145,7 @@ enum lttng_live_iterator_status lttng_live_metadata_update(
                goto end;
        }
 
-       if (!metadata->trace) {
-               trace->new_metadata_needed = false;
-       }
-
-       if (!trace->new_metadata_needed) {
+       if (trace->metadata_stream_state != LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED) {
                goto end;
        }
 
@@ -201,7 +197,14 @@ enum lttng_live_iterator_status lttng_live_metadata_update(
                        BT_COMP_LOGD("Metadata stream was closed by the Relay, the trace is no longer active: "
                                "trace-id=%"PRIu64", metadata-stream-id=%"PRIu64,
                                trace->id, metadata->stream_id);
+                       /*
+                        * The stream was closed and we received everything
+                        * there was to receive for this metadata stream.
+                        * We go on with the decoding of what we received. So
+                        * that data stream can be decoded.
+                        */
                        keep_receiving = false;
+                       trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_CLOSED;
                        break;
                case LTTNG_LIVE_GET_ONE_METADATA_STATUS_ERROR:
                        BT_COMP_LOGE_APPEND_CAUSE(self_comp,
@@ -213,15 +216,6 @@ enum lttng_live_iterator_status lttng_live_metadata_update(
                }
        }
 
-       /*
-        * A closed metadata stream means the trace is no longer active. Return
-        * _END so that the caller can remove the trace from its list.
-        */
-       if (metadata_status == LTTNG_LIVE_GET_ONE_METADATA_STATUS_CLOSED) {
-               status = LTTNG_LIVE_ITERATOR_STATUS_END;
-               goto end;
-       }
-
        /* The memory buffer `metadata_buf` contains all the metadata. */
        if (bt_close_memstream(&metadata_buf, &size, fp)) {
                BT_COMP_LOGW_ERRNO("Metadata bt_close_memstream", ".");
@@ -234,7 +228,9 @@ enum lttng_live_iterator_status lttng_live_metadata_update(
                        status = LTTNG_LIVE_ITERATOR_STATUS_AGAIN;
                        goto end;
                }
-               trace->new_metadata_needed = false;
+
+               /* The relay sent zero bytes of metdata. */
+               trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NOT_NEEDED;
                goto end;
        }
 
@@ -294,7 +290,9 @@ enum lttng_live_iterator_status lttng_live_metadata_update(
                        trace->clock_class =
                                borrow_any_clock_class(trace->trace_class);
                }
-               trace->new_metadata_needed = false;
+
+               /* The metadata was updated succesfully. */
+               trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NOT_NEEDED;
 
                break;
        case CTF_METADATA_DECODER_STATUS_INCOMPLETE:
@@ -358,7 +356,6 @@ int lttng_live_metadata_create_stream(struct lttng_live_session *session,
                        "Failed to borrow trace");
                goto error;
        }
-       metadata->trace = trace;
        trace->metadata = metadata;
        return 0;
 

diff --git a/src/plugins/ctf/lttng-live/viewer-connection.c b/src/plugins/ctf/lttng-live/viewer-connection.c
index 2cf23d171ce8ec326654af8d3400fc6f97a0c96c..4139546611fd20a865cd8ba676fd8478706f003d 100644 (file)
--- a/src/plugins/ctf/lttng-live/viewer-connection.c
+++ b/src/plugins/ctf/lttng-live/viewer-connection.c
@@ -1443,7 +1443,7 @@ enum lttng_live_iterator_status lttng_live_get_next_index(
 
                if (flags & LTTNG_VIEWER_FLAG_NEW_METADATA) {
                        BT_COMP_LOGD("Received get_next_index response: new metadata needed");
-                       trace->new_metadata_needed = true;
+                       trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED;
                }
                if (flags & LTTNG_VIEWER_FLAG_NEW_STREAM) {
                        BT_COMP_LOGD("Received get_next_index response: new streams needed");
@@ -1556,7 +1556,7 @@ enum ctf_msg_iter_medium_status lttng_live_get_stream_bytes(
        case LTTNG_VIEWER_GET_PACKET_ERR:
                if (flags & LTTNG_VIEWER_FLAG_NEW_METADATA) {
                        BT_COMP_LOGD("get_data_packet: new metadata needed, try again later");
-                       trace->new_metadata_needed = true;
+                       trace->metadata_stream_state = LTTNG_LIVE_METADATA_STREAM_STATE_NEEDED;
                }
                if (flags & LTTNG_VIEWER_FLAG_NEW_STREAM) {
                        BT_COMP_LOGD("get_data_packet: new streams needed, try again later");