From: Francis Deslauriers Date: Thu, 7 Nov 2019 20:42:19 +0000 (-0500) Subject: Fix: src.ctf.lttng-live: decode metadata even on _STATUS_CLOSED X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=76bbaebcbc68c8765517a3db2c293fad83a5d162 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 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 64148053..79f6c3fa 100644 --- 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 82ebbff9..078fe94d 100644 --- 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 1447cc5d..58131f82 100644 --- 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 d347534d..175b8071 100644 --- 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 2cf23d17..41395466 100644 --- 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");