From cb523e0290a439cf57fa7823ffa78803500ba4c3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 12 Oct 2018 19:49:42 -0400 Subject: [PATCH] Fix relayd: stream index file created in the wrong directory MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This fix addresses an issue that can cause a stream's index file to be created in the wrong trace archive chunk's directory. The data connection creates a stream's first index file. This can happen _after_ a ROTATE_STREAM command. More specifically, the data of the first packet of a stream can be received after a ROTATE_STREAM command. The ROTATE_STREAM command changes the streams path_name to point to the "next" chunk. If a rotation is pending for a stream, as indicated by "rotate_at_seq_num != -1ULL", it means that we are still receiving data that belongs in the stream's former path. In fact, we may have never received any data for this stream at this point. In this specific case, we must ensure that the index file is created in the streams's former path, "prev_path_name", on reception of the first packet's data on the data connection. All other rotations beyond the first one are not affected by this problem since the actual rotation operation creates the new chunk's index file. Signed-off-by: Jérémie Galarneau --- src/bin/lttng-relayd/main.c | 41 ++++++++++++++++++++++++++++++----- src/bin/lttng-relayd/stream.c | 2 ++ src/bin/lttng-relayd/stream.h | 14 ++++++++++++ src/common/index/index.c | 2 +- src/common/index/index.h | 2 +- 5 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index cbe1a607c..cf6e01ee0 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -1539,7 +1539,8 @@ end: * Return 0 on success, -1 on error. */ static -int create_rotate_index_file(struct relay_stream *stream) +int create_rotate_index_file(struct relay_stream *stream, + const char *stream_path) { int ret; uint32_t major, minor; @@ -1551,7 +1552,7 @@ int create_rotate_index_file(struct relay_stream *stream) } major = stream->trace->session->major; minor = stream->trace->session->minor; - stream->index_file = lttng_index_file_create(stream->path_name, + stream->index_file = lttng_index_file_create(stream_path, stream->channel_name, -1, -1, stream->tracefile_size, tracefile_array_get_file_index_head(stream->tfa), @@ -1587,7 +1588,7 @@ int do_rotate_stream(struct relay_stream *stream) /* Rotate also the index if the stream is not a metadata stream. */ if (!stream->is_metadata) { - ret = create_rotate_index_file(stream); + ret = create_rotate_index_file(stream, stream->path_name); if (ret < 0) { ERR("Failed to rotate index file"); goto end; @@ -1709,7 +1710,7 @@ int rotate_truncate_stream(struct relay_stream *stream) goto end; } - ret = create_rotate_index_file(stream); + ret = create_rotate_index_file(stream, stream->path_name); if (ret < 0) { ERR("Rotate stream index file"); goto end; @@ -2526,7 +2527,8 @@ static int relay_rotate_session_stream(const struct lttcomm_relayd_hdr *recv_hdr * Update the trace path (just the folder, the stream name does not * change). */ - free(stream->path_name); + free(stream->prev_path_name); + stream->prev_path_name = stream->path_name; stream->path_name = create_output_path(new_path_view.data); if (!stream->path_name) { ERR("Failed to create a new output path"); @@ -3250,7 +3252,34 @@ static int handle_index_data(struct relay_stream *stream, uint64_t net_seq_num, } if (rotate_index || !stream->index_file) { - ret = create_rotate_index_file(stream); + const char *stream_path; + + /* + * The data connection creates the stream's first index file. + * + * This can happen _after_ a ROTATE_STREAM command. In + * other words, the data of the first packet of this stream + * can be received after a ROTATE_STREAM command. + * + * The ROTATE_STREAM command changes the stream's path_name + * to point to the "next" chunk. If a rotation is pending for + * this stream, as indicated by "rotate_at_seq_num != -1ULL", + * it means that we are still receiving data that belongs in the + * stream's former path. + * + * In this very specific case, we must ensure that the index + * file is created in the streams's former path, + * "prev_path_name". + * + * All other rotations beyond the first one are not affected + * by this problem since the actual rotation operation creates + * the new chunk's index file. + */ + stream_path = stream->rotate_at_seq_num == -1ULL ? + stream->path_name: + stream->prev_path_name; + + ret = create_rotate_index_file(stream, stream_path); if (ret < 0) { ERR("Failed to rotate index"); /* Put self-ref for this index due to error. */ diff --git a/src/bin/lttng-relayd/stream.c b/src/bin/lttng-relayd/stream.c index ac880e7e5..41d44a585 100644 --- a/src/bin/lttng-relayd/stream.c +++ b/src/bin/lttng-relayd/stream.c @@ -89,6 +89,7 @@ struct relay_stream *stream_create(struct ctf_trace *trace, stream->tracefile_size = tracefile_size; stream->tracefile_count = tracefile_count; stream->path_name = path_name; + stream->prev_path_name = NULL; stream->channel_name = channel_name; stream->rotate_at_seq_num = -1ULL; lttng_ht_node_init_u64(&stream->node, stream->stream_handle); @@ -255,6 +256,7 @@ static void stream_destroy(struct relay_stream *stream) tracefile_array_destroy(stream->tfa); } free(stream->path_name); + free(stream->prev_path_name); free(stream->channel_name); free(stream); } diff --git a/src/bin/lttng-relayd/stream.h b/src/bin/lttng-relayd/stream.h index ae75dacc3..dcdacfbb6 100644 --- a/src/bin/lttng-relayd/stream.h +++ b/src/bin/lttng-relayd/stream.h @@ -65,6 +65,20 @@ struct relay_stream { struct lttng_index_file *index_file; char *path_name; + /* + * prev_path_name is only used for session rotation support. + * It is essentially used to work around the fact that index + * files are always created from the 'data' connection. + * + * Hence, it is possible to receive a ROTATE_STREAM command + * which affects the stream's path_name before the creation of + * an index file. In this situation, the index file of the + * 'previous' chunk would be created in the new destination folder. + * + * It would then be unlinked when the actual index of the new chunk + * is created. + */ + char *prev_path_name; char *channel_name; /* On-disk circular buffer of tracefiles. */ diff --git a/src/common/index/index.c b/src/common/index/index.c index 12d54e006..fae1bfd4b 100644 --- a/src/common/index/index.c +++ b/src/common/index/index.c @@ -35,7 +35,7 @@ * * Return allocated struct lttng_index_file, NULL on error. */ -struct lttng_index_file *lttng_index_file_create(char *path_name, +struct lttng_index_file *lttng_index_file_create(const char *path_name, char *stream_name, int uid, int gid, uint64_t size, uint64_t count, uint32_t major, uint32_t minor) { diff --git a/src/common/index/index.h b/src/common/index/index.h index 7020936b6..83e61f648 100644 --- a/src/common/index/index.h +++ b/src/common/index/index.h @@ -37,7 +37,7 @@ struct lttng_index_file { * create and open have refcount of 1. Use put to decrement the * refcount. Destroys when reaching 0. Use "get" to increment refcount. */ -struct lttng_index_file *lttng_index_file_create(char *path_name, +struct lttng_index_file *lttng_index_file_create(const char *path_name, char *stream_name, int uid, int gid, uint64_t size, uint64_t count, uint32_t major, uint32_t minor); struct lttng_index_file *lttng_index_file_open(const char *path_name, -- 2.34.1