Re: [lttng-dev] [PATCH babeltrace] Fix: lttng-live discarded event count after inactivity

[Jérémie Galarneau]

Merged in stable-1.5 and stable-1.4. Thanks!

Jérémie

On 9 January 2018 at 17:12, Julien Desfossez  wrote:
> When a stream is inactive, the consumer produces fake indexes which are
> beacons to let the viewer know a stream has not produced any data up to
> a certain timestamp. These beacon are actually real packet indexes with
> all the fields set to 0 except for the timestamp_end. Currently we keep
> these beacons just like we keep real indexes. The problem is that when
> we switch packet, we compare the events_discarded field in the index we
> just received with the same field in the previous index. In the case
> where a stream has been inactive, we have received inactivity beacons,
> and set the discarded_event field to 0, so the difference with the next
> real index might be wrong.
>
> In fact, since the inactivity beacons are only used to push the
> timestamp end of a stream, we don't need to keep them and we actually
> need to keep most of the data from the real previous index. So we now
> copy the entire prev_index into the cur_index when we receive an
> inactivity beacon. We could refactor the code to avoid performing the
> pointer swap of cur and prev indexes, but this implies a redesign of
> much of the packet switching code which would affect other code paths.
>
> Signed-off-by: Julien Desfossez 
> ---
>  formats/lttng-live/lttng-live-comm.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/formats/lttng-live/lttng-live-comm.c 
> b/formats/lttng-live/lttng-live-comm.c
> index 77bf34a..3ffa257 100644
> --- a/formats/lttng-live/lttng-live-comm.c
> +++ b/formats/lttng-live/lttng-live-comm.c
> @@ -1275,6 +1275,7 @@ retry:
> pos->offset = 0;
> }
>
> +   /* Beacon packet index */
> if (cur_index->content_size == 0) {
> if (file_stream->parent.stream_class) {
> file_stream->parent.cycles_timestamp =
> @@ -1282,8 +1283,23 @@ retry:
> file_stream->parent.real_timestamp = 
> ctf_get_real_timestamp(
> _stream->parent,
> cur_index->ts_cycles.timestamp_end);
> +
> +   /*
> +* Duplicate the data from the previous index, because
> +* the one we just received is only a beacon with no
> +* relevant information except the timestamp_end. We
> +* don't need to keep this timestamp_end because we 
> already
> +* updated the file_stream timestamps, so we only need
> +* to keep the last real index data as prev_index. 
> That
> +* way, we keep the original prev timestamps and
> +* discarded events counter. This is the same 
> behaviour
> +* as if we were reading a local trace, we would not
> +* have fake indexes between real indexes.
> +*/
> +   memcpy(cur_index, prev_index, sizeof(struct 
> packet_index));
> }
> } else {
> +   /* Real packet index */
> if (file_stream->parent.stream_class) {
> /* Convert the timestamps and append to the 
> real_index. */
> cur_index->ts_real.timestamp_begin = 
> ctf_get_real_timestamp(
> --
> 2.7.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH babeltrace] Fix: lttng-live discarded event count after inactivity

[Mathieu Desnoyers]

- On Jan 9, 2018, at 5:12 PM, Julien Desfossez jdesfos...@efficios.com 
wrote:

> When a stream is inactive, the consumer produces fake indexes which are
> beacons to let the viewer know a stream has not produced any data up to
> a certain timestamp. These beacon are actually real packet indexes with
> all the fields set to 0 except for the timestamp_end. Currently we keep
> these beacons just like we keep real indexes. The problem is that when
> we switch packet, we compare the events_discarded field in the index we
> just received with the same field in the previous index. In the case
> where a stream has been inactive, we have received inactivity beacons,
> and set the discarded_event field to 0, so the difference with the next
> real index might be wrong.
> 
> In fact, since the inactivity beacons are only used to push the
> timestamp end of a stream, we don't need to keep them and we actually
> need to keep most of the data from the real previous index. So we now
> copy the entire prev_index into the cur_index when we receive an
> inactivity beacon. We could refactor the code to avoid performing the
> pointer swap of cur and prev indexes, but this implies a redesign of
> much of the packet switching code which would affect other code paths.
> 
> Signed-off-by: Julien Desfossez 

Acked-by: Mathieu Desnoyers 

> ---
> formats/lttng-live/lttng-live-comm.c | 16 
> 1 file changed, 16 insertions(+)
> 
> diff --git a/formats/lttng-live/lttng-live-comm.c
> b/formats/lttng-live/lttng-live-comm.c
> index 77bf34a..3ffa257 100644
> --- a/formats/lttng-live/lttng-live-comm.c
> +++ b/formats/lttng-live/lttng-live-comm.c
> @@ -1275,6 +1275,7 @@ retry:
>   pos->offset = 0;
>   }
> 
> + /* Beacon packet index */
>   if (cur_index->content_size == 0) {
>   if (file_stream->parent.stream_class) {
>   file_stream->parent.cycles_timestamp =
> @@ -1282,8 +1283,23 @@ retry:
>   file_stream->parent.real_timestamp = 
> ctf_get_real_timestamp(
>   _stream->parent,
>   cur_index->ts_cycles.timestamp_end);
> +
> + /*
> +  * Duplicate the data from the previous index, because
> +  * the one we just received is only a beacon with no
> +  * relevant information except the timestamp_end. We
> +  * don't need to keep this timestamp_end because we 
> already
> +  * updated the file_stream timestamps, so we only need
> +  * to keep the last real index data as prev_index. That
> +  * way, we keep the original prev timestamps and
> +  * discarded events counter. This is the same behaviour
> +  * as if we were reading a local trace, we would not
> +  * have fake indexes between real indexes.
> +  */
> + memcpy(cur_index, prev_index, sizeof(struct 
> packet_index));
>   }
>   } else {
> + /* Real packet index */
>   if (file_stream->parent.stream_class) {
>   /* Convert the timestamps and append to the real_index. 
> */
>   cur_index->ts_real.timestamp_begin = 
> ctf_get_real_timestamp(
> --
> 2.7.4
> 
> ___
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH babeltrace] Fix: lttng-live discarded event count after inactivity

[Jonathan Rajotte-Julien]

On Tue, Jan 09, 2018 at 05:12:54PM -0500, Julien Desfossez wrote:
> When a stream is inactive, the consumer produces fake indexes which are
> beacons to let the viewer know a stream has not produced any data up to
> a certain timestamp. These beacon are actually real packet indexes with
> all the fields set to 0 except for the timestamp_end. Currently we keep
> these beacons just like we keep real indexes. The problem is that when
> we switch packet, we compare the events_discarded field in the index we
> just received with the same field in the previous index. In the case
> where a stream has been inactive, we have received inactivity beacons,
> and set the discarded_event field to 0, so the difference with the next
> real index might be wrong.
> 
> In fact, since the inactivity beacons are only used to push the
> timestamp end of a stream, we don't need to keep them and we actually
> need to keep most of the data from the real previous index. So we now
> copy the entire prev_index into the cur_index when we receive an
> inactivity beacon. We could refactor the code to avoid performing the
> pointer swap of cur and prev indexes, but this implies a redesign of
> much of the packet switching code which would affect other code paths.
> 

Tested-by: Jonathan Rajotte-Julien 

> Signed-off-by: Julien Desfossez 
> ---
>  formats/lttng-live/lttng-live-comm.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/formats/lttng-live/lttng-live-comm.c 
> b/formats/lttng-live/lttng-live-comm.c
> index 77bf34a..3ffa257 100644
> --- a/formats/lttng-live/lttng-live-comm.c
> +++ b/formats/lttng-live/lttng-live-comm.c
> @@ -1275,6 +1275,7 @@ retry:
>   pos->offset = 0;
>   }
>  
> + /* Beacon packet index */
>   if (cur_index->content_size == 0) {
>   if (file_stream->parent.stream_class) {
>   file_stream->parent.cycles_timestamp =
> @@ -1282,8 +1283,23 @@ retry:
>   file_stream->parent.real_timestamp = 
> ctf_get_real_timestamp(
>   _stream->parent,
>   cur_index->ts_cycles.timestamp_end);
> +
> + /*
> +  * Duplicate the data from the previous index, because
> +  * the one we just received is only a beacon with no
> +  * relevant information except the timestamp_end. We
> +  * don't need to keep this timestamp_end because we 
> already
> +  * updated the file_stream timestamps, so we only need
> +  * to keep the last real index data as prev_index. That
> +  * way, we keep the original prev timestamps and
> +  * discarded events counter. This is the same behaviour
> +  * as if we were reading a local trace, we would not
> +  * have fake indexes between real indexes.
> +  */
> + memcpy(cur_index, prev_index, sizeof(struct 
> packet_index));
>   }
>   } else {
> + /* Real packet index */
>   if (file_stream->parent.stream_class) {
>   /* Convert the timestamps and append to the real_index. 
> */
>   cur_index->ts_real.timestamp_begin = 
> ctf_get_real_timestamp(
> -- 
> 2.7.4
> 

-- 
Jonathan Rajotte-Julien
EfficiOS
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] [PATCH babeltrace] Fix: lttng-live discarded event count after inactivity

[Julien Desfossez]

When a stream is inactive, the consumer produces fake indexes which are
beacons to let the viewer know a stream has not produced any data up to
a certain timestamp. These beacon are actually real packet indexes with
all the fields set to 0 except for the timestamp_end. Currently we keep
these beacons just like we keep real indexes. The problem is that when
we switch packet, we compare the events_discarded field in the index we
just received with the same field in the previous index. In the case
where a stream has been inactive, we have received inactivity beacons,
and set the discarded_event field to 0, so the difference with the next
real index might be wrong.

In fact, since the inactivity beacons are only used to push the
timestamp end of a stream, we don't need to keep them and we actually
need to keep most of the data from the real previous index. So we now
copy the entire prev_index into the cur_index when we receive an
inactivity beacon. We could refactor the code to avoid performing the
pointer swap of cur and prev indexes, but this implies a redesign of
much of the packet switching code which would affect other code paths.

Signed-off-by: Julien Desfossez 
---
 formats/lttng-live/lttng-live-comm.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/formats/lttng-live/lttng-live-comm.c 
b/formats/lttng-live/lttng-live-comm.c
index 77bf34a..3ffa257 100644
--- a/formats/lttng-live/lttng-live-comm.c
+++ b/formats/lttng-live/lttng-live-comm.c
@@ -1275,6 +1275,7 @@ retry:
pos->offset = 0;
}
 
+   /* Beacon packet index */
if (cur_index->content_size == 0) {
if (file_stream->parent.stream_class) {
file_stream->parent.cycles_timestamp =
@@ -1282,8 +1283,23 @@ retry:
file_stream->parent.real_timestamp = 
ctf_get_real_timestamp(
_stream->parent,
cur_index->ts_cycles.timestamp_end);
+
+   /*
+* Duplicate the data from the previous index, because
+* the one we just received is only a beacon with no
+* relevant information except the timestamp_end. We
+* don't need to keep this timestamp_end because we 
already
+* updated the file_stream timestamps, so we only need
+* to keep the last real index data as prev_index. That
+* way, we keep the original prev timestamps and
+* discarded events counter. This is the same behaviour
+* as if we were reading a local trace, we would not
+* have fake indexes between real indexes.
+*/
+   memcpy(cur_index, prev_index, sizeof(struct 
packet_index));
}
} else {
+   /* Real packet index */
if (file_stream->parent.stream_class) {
/* Convert the timestamps and append to the real_index. 
*/
cur_index->ts_real.timestamp_begin = 
ctf_get_real_timestamp(
-- 
2.7.4

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev