Fix: consumer: rotation error return codes

The return codes of rotation commands from consumerd are not
in sync with reality. Some are simply copy-pasted from old code.

Add new return codes to describe each error situation, and split the
"channel lookup" error from other errors so sessiond can distinguish
between an error caused by an exiting application (per-pid buffers)
(LTTCOMM_CONSUMERD_CHAN_NOT_FOUND) and an actual error while performing
the command.

Move the channel lookup into the rotation caller, because it is used by
two sub-functions: we don't want to lookup to succeed for the first and
then fail for the second.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c
index b07939113cfe2e5bb1d87ec23a9da745d7a22508..c7b77010c76bf4a157941f1d35ade3e00286e4a9 100644 (file)
--- a/src/common/consumer/consumer.c
+++ b/src/common/consumer/consumer.c
@@ -3896,15 +3896,16 @@ end:
  * is already at the rotate position (produced == consumed), we flag it as
  * ready for rotation. The rotation of ready streams occurs after we have
  * replied to the session daemon that we have finished sampling the positions.
+ * Must be called with RCU read-side lock held to ensure existence of channel.
  *
  * Returns 0 on success, < 0 on error
  */
-int lttng_consumer_rotate_channel(uint64_t key, const char *path,
-               uint64_t relayd_id, uint32_t metadata, uint64_t new_chunk_id,
+int lttng_consumer_rotate_channel(struct lttng_consumer_channel *channel,
+               uint64_t key, const char *path, uint64_t relayd_id,
+               uint32_t metadata, uint64_t new_chunk_id,
                struct lttng_consumer_local_data *ctx)
 {
        int ret;
-       struct lttng_consumer_channel *channel;
        struct lttng_consumer_stream *stream;
        struct lttng_ht_iter iter;
        struct lttng_ht *ht = consumer_data.stream_per_chan_id_ht;
@@ -3913,13 +3914,6 @@ int lttng_consumer_rotate_channel(uint64_t key, const char *path,
 
        rcu_read_lock();
 
-       channel = consumer_find_channel(key);
-       if (!channel) {
-               ERR("No channel found for key %" PRIu64, key);
-               ret = -1;
-               goto end;
-       }
-
        pthread_mutex_lock(&channel->lock);
        channel->current_chunk_id = new_chunk_id;
 
@@ -4232,14 +4226,15 @@ error:
  * This is especially important for low throughput streams that have already
  * been consumed, we cannot wait for their next packet to perform the
  * rotation.
+ * Need to be called with RCU read-side lock held to ensure existence of
+ * channel.
  *
  * Returns 0 on success, < 0 on error
  */
-int lttng_consumer_rotate_ready_streams(uint64_t key,
-               struct lttng_consumer_local_data *ctx)
+int lttng_consumer_rotate_ready_streams(struct lttng_consumer_channel *channel,
+               uint64_t key, struct lttng_consumer_local_data *ctx)
 {
        int ret;
-       struct lttng_consumer_channel *channel;
        struct lttng_consumer_stream *stream;
        struct lttng_ht_iter iter;
        struct lttng_ht *ht = consumer_data.stream_per_chan_id_ht;
@@ -4248,13 +4243,6 @@ int lttng_consumer_rotate_ready_streams(uint64_t key,
 
        DBG("Consumer rotate ready streams in channel %" PRIu64, key);
 
-       channel = consumer_find_channel(key);
-       if (!channel) {
-               ERR("No channel found for key %" PRIu64, key);
-               ret = -1;
-               goto end;
-       }
-
        cds_lfht_for_each_entry_duplicate(ht->ht,
                        ht->hash_fct(&channel->key, lttng_ht_seed),
                        ht->match_fct, &channel->key, &iter.iter,

diff --git a/src/common/consumer/consumer.h b/src/common/consumer/consumer.h
index 2c927bbc2ca0b745890130ade943e15d818054e7..b940036f6f074ed60cbfeac3daa6f29c1a85095e 100644 (file)
--- a/src/common/consumer/consumer.h
+++ b/src/common/consumer/consumer.h
@@ -834,14 +834,15 @@ void consumer_del_stream_for_data(struct lttng_consumer_stream *stream);
 void consumer_add_metadata_stream(struct lttng_consumer_stream *stream);
 void consumer_del_stream_for_metadata(struct lttng_consumer_stream *stream);
 int consumer_create_index_file(struct lttng_consumer_stream *stream);
-int lttng_consumer_rotate_channel(uint64_t key, const char *path,
-               uint64_t relayd_id, uint32_t metadata,
-               uint64_t new_chunk_id, struct lttng_consumer_local_data *ctx);
+int lttng_consumer_rotate_channel(struct lttng_consumer_channel *channel,
+               uint64_t key, const char *path, uint64_t relayd_id,
+               uint32_t metadata, uint64_t new_chunk_id,
+               struct lttng_consumer_local_data *ctx);
 int lttng_consumer_stream_is_rotate_ready(struct lttng_consumer_stream *stream);
 int lttng_consumer_rotate_stream(struct lttng_consumer_local_data *ctx,
                struct lttng_consumer_stream *stream, bool *rotated);
-int lttng_consumer_rotate_ready_streams(uint64_t key,
-               struct lttng_consumer_local_data *ctx);
+int lttng_consumer_rotate_ready_streams(struct lttng_consumer_channel *channel,
+               uint64_t key, struct lttng_consumer_local_data *ctx);
 int lttng_consumer_rotate_rename(const char *current_path, const char *new_path,
                uid_t uid, gid_t gid, uint64_t relayd_id);
 int lttng_consumer_check_rotation_pending_local(uint64_t session_id,

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index c223fa395af172525985e92bdaf8b39058647f6d..68d69bb2a658658ef37dbd83b9c2094a40a4924a 100644 (file)
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -1084,35 +1084,44 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
        }
        case LTTNG_CONSUMER_ROTATE_CHANNEL:
        {
-               DBG("Consumer rotate channel %" PRIu64, msg.u.rotate_channel.key);
+               struct lttng_consumer_channel *channel;
+               uint64_t key = msg.u.rotate_channel.key;
 
-               /*
-                * Sample the rotate position of all the streams in this channel.
-                */
-               ret = lttng_consumer_rotate_channel(msg.u.rotate_channel.key,
-                               msg.u.rotate_channel.pathname,
-                               msg.u.rotate_channel.relayd_id,
-                               msg.u.rotate_channel.metadata,
-                               msg.u.rotate_channel.new_chunk_id,
-                               ctx);
-               if (ret < 0) {
-                       ERR("Rotate channel failed");
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
-               }
+               DBG("Consumer rotate channel %" PRIu64, key);
 
-               health_code_update();
+               channel = consumer_find_channel(key);
+               if (!channel) {
+                       ERR("Channel %" PRIu64 " not found", key);
+                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+               } else {
+                       /*
+                        * Sample the rotate position of all the streams in this channel.
+                        */
+                       ret = lttng_consumer_rotate_channel(channel, key,
+                                       msg.u.rotate_channel.pathname,
+                                       msg.u.rotate_channel.relayd_id,
+                                       msg.u.rotate_channel.metadata,
+                                       msg.u.rotate_channel.new_chunk_id,
+                                       ctx);
+                       if (ret < 0) {
+                               ERR("Rotate channel failed");
+                               ret_code = LTTCOMM_CONSUMERD_ROTATION_FAIL;
+                       }
 
+                       health_code_update();
+               }
                ret = consumer_send_status_msg(sock, ret_code);
                if (ret < 0) {
                        /* Somehow, the session daemon is not responding anymore. */
                        goto end_nosignal;
                }
-
-               /* Rotate the streams that are ready right now. */
-               ret = lttng_consumer_rotate_ready_streams(
-                               msg.u.rotate_channel.key, ctx);
-               if (ret < 0) {
-                       ERR("Rotate ready streams failed");
+               if (channel) {
+                       /* Rotate the streams that are ready right now. */
+                       ret = lttng_consumer_rotate_ready_streams(
+                                       channel, key, ctx);
+                       if (ret < 0) {
+                               ERR("Rotate ready streams failed");
+                       }
                }
 
                break;
@@ -1130,7 +1139,7 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.rotate_rename.relayd_id);
                if (ret < 0) {
                        ERR("Rotate rename failed");
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATE_RENAME_FAILED;
                }
 
                health_code_update();
@@ -1154,7 +1163,7 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.check_rotation_pending_local.chunk_id);
                if (pending < 0) {
                        ERR("Local rotation pending check failed with code %i", pending);
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATION_PENDING_LOCAL_FAILED;
                } else {
                        pending_reply = !!pending;
                }
@@ -1198,7 +1207,7 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.check_rotation_pending_relay.chunk_id);
                if (pending < 0) {
                        ERR("Relayd rotation pending check failed with code %i", pending);
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATION_PENDING_RELAY_FAILED;
                } else {
                        pending_reply = !!pending;
                }
@@ -1240,7 +1249,7 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.mkdir.relayd_id);
                if (ret < 0) {
                        ERR("consumer mkdir failed");
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_MKDIR_FAILED;
                }
 
                health_code_update();

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 72aa61e0beab45f16e97dff3da2093c1bb12d687..5b70a4ac5baadc40d6ea64928e3d88b58be984b5 100644 (file)
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -164,6 +164,11 @@ enum lttcomm_return_code {
        LTTCOMM_CONSUMERD_CHANNEL_FAIL,             /* Channel creation failed. */
        LTTCOMM_CONSUMERD_CHAN_NOT_FOUND,           /* Channel not found. */
        LTTCOMM_CONSUMERD_ALREADY_SET,              /* Resource already set. */
+       LTTCOMM_CONSUMERD_ROTATION_FAIL,            /* Rotation has failed. */
+       LTTCOMM_CONSUMERD_ROTATE_RENAME_FAILED,     /* Rotation rename has failed. */
+       LTTCOMM_CONSUMERD_ROTATION_PENDING_LOCAL_FAILED, /* Rotation pending relay failed. */
+       LTTCOMM_CONSUMERD_ROTATION_PENDING_RELAY_FAILED, /* Rotation pending relay failed. */
+       LTTCOMM_CONSUMERD_MKDIR_FAILED,             /* mkdir has failed. */
 
        /* MUST be last element */
        LTTCOMM_NR,                                             /* Last element */

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index a81a497a9f8a8fa042ba28f29bbf0e86e73dc92e..5e34b1a36919773da0b51c751cc3ceb38b6514f9 100644 (file)
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -1931,22 +1931,31 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
        }
        case LTTNG_CONSUMER_ROTATE_CHANNEL:
        {
-               /*
-                * Sample the rotate position of all the streams in this channel.
-                */
-               ret = lttng_consumer_rotate_channel(msg.u.rotate_channel.key,
-                               msg.u.rotate_channel.pathname,
-                               msg.u.rotate_channel.relayd_id,
-                               msg.u.rotate_channel.metadata,
-                               msg.u.rotate_channel.new_chunk_id,
-                               ctx);
-               if (ret < 0) {
-                       ERR("Rotate channel failed");
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
-               }
+               struct lttng_consumer_channel *channel;
+               uint64_t key = msg.u.rotate_channel.key;
 
-               health_code_update();
+               channel = consumer_find_channel(key);
+               if (!channel) {
+                       DBG("Channel %" PRIu64 " not found", key);
+                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+               } else {
+                       /*
+                        * Sample the rotate position of all the streams in
+                        * this channel.
+                        */
+                       ret = lttng_consumer_rotate_channel(channel, key,
+                                       msg.u.rotate_channel.pathname,
+                                       msg.u.rotate_channel.relayd_id,
+                                       msg.u.rotate_channel.metadata,
+                                       msg.u.rotate_channel.new_chunk_id,
+                                       ctx);
+                       if (ret < 0) {
+                               ERR("Rotate channel failed");
+                               ret_code = LTTCOMM_CONSUMERD_ROTATION_FAIL;
+                       }
 
+                       health_code_update();
+               }
                ret = consumer_send_status_msg(sock, ret_code);
                if (ret < 0) {
                        /* Somehow, the session daemon is not responding anymore. */
@@ -1960,10 +1969,12 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                 * handle this, but it needs to be after the
                 * consumer_send_status_msg() call.
                 */
-               ret = lttng_consumer_rotate_ready_streams(
-                               msg.u.rotate_channel.key, ctx);
-               if (ret < 0) {
-                       ERR("Rotate channel failed");
+               if (channel) {
+                       ret = lttng_consumer_rotate_ready_streams(
+                                       channel, key, ctx);
+                       if (ret < 0) {
+                               ERR("Rotate channel failed");
+                       }
                }
                break;
        }
@@ -1978,7 +1989,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.rotate_rename.relayd_id);
                if (ret < 0) {
                        ERR("Rotate rename failed");
-                       ret_code = LTTCOMM_CONSUMERD_RELAYD_FAIL;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATE_RENAME_FAILED;
                }
 
                health_code_update();
@@ -2002,7 +2013,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.check_rotation_pending_local.chunk_id);
                if (pending < 0) {
                        ERR("Local rotation pending check failed with code %i", pending);
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATION_PENDING_LOCAL_FAILED;
                } else {
                        pending_reply = !!pending;
                }
@@ -2046,7 +2057,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.check_rotation_pending_relay.chunk_id);
                if (pending < 0) {
                        ERR("Relayd rotation pending check failed with code %i", pending);
-                       ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND;
+                       ret_code = LTTCOMM_CONSUMERD_ROTATION_PENDING_RELAY_FAILED;
                } else {
                        pending_reply = !!pending;
                }
@@ -2088,7 +2099,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.mkdir.relayd_id);
                if (ret < 0) {
                        ERR("consumer mkdir failed");
-                       ret_code = LTTCOMM_CONSUMERD_RELAYD_FAIL;
+                       ret_code = LTTCOMM_CONSUMERD_MKDIR_FAILED;
                }
 
                health_code_update();