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

[Julien Desfossez]

>>> 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.

Publishing the stream does not take a new reference on the stream, it
just adds it in the stream_list so it becomes visible in the viewer
thread. So it does not change the lifetime of the stream. And if the
stream is not published when we remove it we don't try to unpublish it.

>>> 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.

It looks good.

Thanks,

Julien