[lttng-dev] [PATCH lttng-tools v2] Fix: wrong use of the relay_streams_sent in snapshot

[Mathieu Desnoyers]

----- On Aug 24, 2017, at 12:35 PM, Julien Desfossez jdesfossez at efficios.com wrote:

>> If you remove this, I think the streams will never get published
>> within the relayd. (publish_connection_local_streams()). Is this
>> an expected side-effect ? It should be documented in the changelog.
>> My guess is that we indeed don't want to publish the snapshot
>> streams to the viewers.
> Indeed, a snapshot session cannot be a live session, so we don't
> want/need to publish those streams. Also, sending this message every
> time we send a stream is a wrong usage of the command.

You'll need to ensure that we are not freeing the streams too
quickly or leaking them in relayd after your change.

> 
>> The reason for doing this change should also be documented. What
>> behavior is unwanted here from a relayd perspective ?
> We are sending this message to the relay (and waiting for the
> confirmation) before taking the snapshot of each stream. So, in addition
> to being wrong and useless, it adds a considerable delay before taking
> the snapshot of each stream.
> 
> Do you agree ?

It looks like a good idea to remove it. I just want us to make sure the
snapshot mode does not somehow expect the streams to be published
within other parts of the relayd code.

Thanks,

Mathieu

> 
> Thanks,
> 
> Julien

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com