[lttng-dev] [RFC PATCH lttng-tools] Fix: metadata push -EPIPE should be recoverable

[Mathieu Desnoyers]

We should not have to send any error back to the consumer because the 
app disappearing will eventually close all file descriptors related to the 
communication and notification pipes (sessiond vs apps), as well as all 
stream file descriptors (consumerd vs apps). So the consumerd will 
eventually be notified that the application disappeared, and will therefore 
close the associated streams (in per-pid buffers) in due time. 

Thanks, 

Mathieu 

----- On Jul 6, 2015, at 12:42 PM, HT, Aravind <aravind.ht at gmail.com> wrote: 

> In case of -EPIPE, im thinking if we can send LTTNG_ERR_UND or some other error
> back to the consumer for it to do any relevant clean up.

> On Mon, Jul 6, 2015 at 9:51 PM, Mathieu Desnoyers <
> mathieu.desnoyers at efficios.com > wrote:

>> This return value can be caused by application terminating concurrently
>> (when using per-PID buffers), so it should not make the consumer
>> management thread exit.

>> CC: Aravind HT < aravind.ht at gmail.com >
>> Signed-off-by: Mathieu Desnoyers < mathieu.desnoyers at efficios.com >
>> ---
>> src/bin/lttng-sessiond/ust-app.c | 13 ++++++++++---
>> src/bin/lttng-sessiond/ust-consumer.c | 7 +++++--
>> 2 files changed, 15 insertions(+), 5 deletions(-)

>> diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
>> index c30792c..6f032da 100644
>> --- a/src/bin/lttng-sessiond/ust-app.c
>> +++ b/src/bin/lttng-sessiond/ust-app.c
>> @@ -431,6 +431,9 @@ void delete_ust_app_channel(int sock, struct ust_app_channel
>> *ua_chan,
>> * Must be called with the registry lock held.
>> *
>> * On success, return the len of metadata pushed or else a negative value.
>> + * Returning a -EPIPE return value means we could not send the metadata,
>> + * but it can be caused by recoverable errors (e.g. the application has
>> + * terminated concurrently).
>> */
>> ssize_t ust_app_push_metadata(struct ust_registry_session *registry,
>> struct consumer_socket *socket, int send_zero_data)
>> @@ -454,9 +457,10 @@ ssize_t ust_app_push_metadata(struct ust_registry_session
>> *registry,
>> /*
>> * On a push metadata error either the consumer is dead or the
>> * metadata channel has been destroyed because its endpoint
>> - * might have died (e.g: relayd). If so, the metadata closed
>> - * flag is set to 1 so we deny pushing metadata again which is
>> - * not valid anymore on the consumer side.
>> + * might have died (e.g: relayd), or because the application has
>> + * exited. If so, the metadata closed flag is set to 1 so we
>> + * deny pushing metadata again which is not valid anymore on the
>> + * consumer side.
>> */
>> if (registry->metadata_closed) {
>> return -EPIPE;
>> @@ -547,6 +551,9 @@ error_push:
>> * of socket throughout this function.
>> *
>> * Return 0 on success else a negative error.
>> + * Returning a -EPIPE return value means we could not send the metadata,
>> + * but it can be caused by recoverable errors (e.g. the application has
>> + * terminated concurrently).
>> */
>> static int push_metadata(struct ust_registry_session *registry,
>> struct consumer_output *consumer)
>> diff --git a/src/bin/lttng-sessiond/ust-consumer.c
>> b/src/bin/lttng-sessiond/ust-consumer.c
>> index 78e50df..ad076e3 100644
>> --- a/src/bin/lttng-sessiond/ust-consumer.c
>> +++ b/src/bin/lttng-sessiond/ust-consumer.c
>> @@ -511,12 +511,15 @@ int ust_consumer_metadata_request(struct consumer_socket
>> *socket)
>> pthread_mutex_lock(&ust_reg->lock);
>> ret_push = ust_app_push_metadata(ust_reg, socket, 1);
>> pthread_mutex_unlock(&ust_reg->lock);
>> - if (ret_push < 0) {
>> + if (ret_push == -EPIPE) {
>> + DBG("Application or relay closed while pushing metadata");
>> + } else if (ret_push < 0) {
>> ERR("Pushing metadata");
>> ret = -1;
>> goto end;
>> + } else {
>> + DBG("UST Consumer metadata pushed successfully");
>> }
>> - DBG("UST Consumer metadata pushed successfully");
>> ret = 0;

>> end:
>> --
>> 2.1.4

-- 
Mathieu Desnoyers 
EfficiOS Inc. 
http://www.efficios.com 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20150706/5b1ece7a/attachment.html>