From: Mathieu Desnoyers Date: Wed, 14 Nov 2018 21:11:48 +0000 (-0500) Subject: Fix: consumer: rotation error return codes X-Git-Tag: v2.12.0-rc1~760 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=92b7a7f81a94f7616c155aed8a213edd68937a57;p=lttng-tools.git 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 Signed-off-by: Jérémie Galarneau --- diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c index b07939113..c7b77010c 100644 --- 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 2c927bbc2..b940036f6 100644 --- 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 c223fa395..68d69bb2a 100644 --- 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 72aa61e0b..5b70a4ac5 100644 --- 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 a81a497a9..5e34b1a36 100644 --- 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();