src.ctf.fs: sort inputs paths

Sort input paths to make the behavior of the component more
deterministic.

For example, when there are duplicated packets in the trace and we pick
one copy, this will ensure that we always process the data stream files in
the same order, and that we always pick the same copy of the packet.

In practice, this will reduce the chances of observing different
behaviors on different platforms where glib reports the files of a trace
in different orders.

A test is added to verify that the choice of which copy of a packet to
choose, in case a packet is present in multiple copies, is independent
from the order of the inputs paths passed to the component.

Change-Id: I034ba8c909e76a1e89589e1f0e0f37512c9d88bf
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2396

src/plugins/ctf/fs-src/fs.c

@@ -2045,6 +2045,15 @@ gint compare_ds_file_groups_by_first_path(gconstpointer a, gconstpointer b)
 		first_ds_file_info_b->path->str);
 }
 
+static
+gint compare_strings(gconstpointer p_a, gconstpointer p_b)
+{
+	const char *a = *((const char **) p_a);
+	const char *b = *((const char **) p_b);
+
+	return strcmp(a, b);
+}
+
 int ctf_fs_component_create_ctf_fs_trace(
 		struct ctf_fs_component *ctf_fs,
 		const bt_value *paths_value,
@@ -2055,6 +2064,7 @@ int ctf_fs_component_create_ctf_fs_trace(
 	int ret = 0;
 	uint64_t i;
 	bt_logging_level log_level = ctf_fs->log_level;
+	GPtrArray *paths = NULL;
 	GPtrArray *traces;
 	const char *trace_name;
 
@@ -2068,15 +2078,43 @@ int ctf_fs_component_create_ctf_fs_trace(
 		goto error;
 	}
 
+	paths = g_ptr_array_new_with_free_func(g_free);
+	if (!paths) {
+		BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE(self_comp, self_comp_class,
+			"Failed to allocate a GPtrArray.");
+		goto error;
+	}
+
 	trace_name = trace_name_value ? bt_value_string_get(trace_name_value) : NULL;
 
-	/* Start by creating a separate ctf_fs_trace object for each path. */
+	/*
+	 * Create a sorted array of the paths, to make the execution of this
+	 * component deterministic.
+	 */
 	for (i = 0; i < bt_value_array_get_length(paths_value); i++) {
-		const bt_value *path_value = bt_value_array_borrow_element_by_index_const(paths_value, i);
+		const bt_value *path_value =
+			bt_value_array_borrow_element_by_index_const(paths_value, i);
 		const char *input = bt_value_string_get(path_value);
+		gchar *input_copy;
+
+		input_copy = g_strdup(input);
+		if (!input_copy) {
+			BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE(self_comp, self_comp_class,
+				"Failed to copy a string.");
+			goto error;
+		}
+
+		g_ptr_array_add(paths, input_copy);
+	}
+
+	g_ptr_array_sort(paths, compare_strings);
+
+	/* Create a separate ctf_fs_trace object for each path. */
+	for (i = 0; i < paths->len; i++) {
+		const char *path = g_ptr_array_index(paths, i);
 
 		ret = ctf_fs_component_create_ctf_fs_trace_one_path(ctf_fs,
-			input, trace_name, traces, self_comp, self_comp_class);
+			path, trace_name, traces, self_comp, self_comp_class);
 		if (ret) {
 			goto end;
 		}
@@ -2160,7 +2198,14 @@ int ctf_fs_component_create_ctf_fs_trace(
 	ret = -1;
 
 end:
-	g_ptr_array_free(traces, TRUE);
+	if (traces) {
+		g_ptr_array_free(traces, TRUE);
+	}
+
+	if (paths) {
+		g_ptr_array_free(paths, TRUE);
+	}
+
 	return ret;
 }
 

tests/Makefile.am

@@ -97,6 +97,7 @@ endif
 TESTS_PLUGINS = \
 	plugins/src.ctf.fs/fail/test_fail \
 	plugins/src.ctf.fs/succeed/test_succeed \
+	plugins/src.ctf.fs/test_deterministic_ordering \
 	plugins/sink.ctf.fs/succeed/test_succeed \
 	plugins/sink.text.details/succeed/test_succeed
 

tests/data/ctf-traces/deterministic-ordering/a-corrupted/metadata

@@ -0,0 +1,31 @@
+/* CTF 1.8 */
+
+typealias integer { size = 8; align = 8; signed = false; } := uint8_t;
+
+trace {
+	major = 1;
+	minor = 8;
+	byte_order = be;
+	uuid = "c6e53ddd-925c-4b8f-bd19-acd28af9c4f2";
+
+	packet.header := struct {
+		uint8_t stream_id;
+		uint8_t stream_instance_id;
+	};
+};
+
+stream {
+	id = 0;
+	event.header := struct {
+		uint8_t id;
+	};
+
+	packet.context := struct {
+		uint8_t timestamp_begin;
+	};
+};
+
+event {
+	name = gadoua;
+	id = 1;
+};

tests/data/ctf-traces/deterministic-ordering/b-c.expect

@@ -0,0 +1,7 @@
+[Unknown] {0 0 0} Stream beginning
+[0 0] {0 0 0} Packet beginning
+[0 0] {0 0 0} Event `gadoua` (1)
+[0 0] {0 0 0} Event `gadoua` (1)
+[0 0] {0 0 0} Event `gadoua` (1)
+{0 0 0} Packet end
+[Unknown] {0 0 0} Stream end

tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/metadata

@@ -0,0 +1,31 @@
+/* CTF 1.8 */
+
+typealias integer { size = 8; align = 8; signed = false; } := uint8_t;
+
+trace {
+	major = 1;
+	minor = 8;
+	byte_order = be;
+	uuid = "c6e53ddd-925c-4b8f-bd19-acd28af9c4f2";
+
+	packet.header := struct {
+		uint8_t stream_id;
+		uint8_t stream_instance_id;
+	};
+};
+
+stream {
+	id = 0;
+	event.header := struct {
+		uint8_t id;
+	};
+
+	packet.context := struct {
+		uint8_t timestamp_begin;
+	};
+};
+
+event {
+	name = gadoua;
+	id = 1;
+};

tests/data/ctf-traces/deterministic-ordering/c-corrupted/metadata

@@ -0,0 +1,31 @@
+/* CTF 1.8 */
+
+typealias integer { size = 8; align = 8; signed = false; } := uint8_t;
+
+trace {
+	major = 1;
+	minor = 8;
+	byte_order = be;
+	uuid = "c6e53ddd-925c-4b8f-bd19-acd28af9c4f2";
+
+	packet.header := struct {
+		uint8_t stream_id;
+		uint8_t stream_instance_id;
+	};
+};
+
+stream {
+	id = 0;
+	event.header := struct {
+		uint8_t id;
+	};
+
+	packet.context := struct {
+		uint8_t timestamp_begin;
+	};
+};
+
+event {
+	name = gadoua;
+	id = 1;
+};

tests/plugins/src.ctf.fs/Makefile.am

@@ -6,4 +6,5 @@ dist_check_SCRIPTS = \
 	query/test_query_support_info \
 	query/test_query_support_info.py \
 	query/test_query_trace_info \
-	query/test_query_trace_info.py
+	query/test_query_trace_info.py \
+	test_deterministic_ordering

tests/plugins/src.ctf.fs/test_deterministic_ordering

@@ -0,0 +1,115 @@
+#!/bin/bash
+#
+# Copyright (C) 2019 Efficios, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; only version 2
+# of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+
+# Test the deterministic behavior of the src.ctf.fs component versus the
+# ordering of the given input paths.
+#
+# In presence of multiple copies of the same packet, we want it to pick the
+# copy of the packet to read in a deterministic fashion.
+#
+# This test is written assuming the specific implementation of the src.ctf.fs
+# component class, which sorts its input paths lexicographically.
+#
+# There are three traces (a-corrupted, b-not-corrupted and c-corrupted) with the
+# same UUID and the same packet, except that this packet is corrupted in
+# a-corrupted and c-corrupted. In these cases, there is an event with an
+# invalid id.  When reading these corrupted packets, we expect babeltrace to
+# emit an error.
+#
+# When reading a-corrupted and b-not-corrupted together, the copy of the packet
+# from a-corrupted is read, and babeltrace exits with an error.
+#
+# When reading b-not-corrupted and c-corrupted together, the copy of the packet
+# from b-not-corrupted is read, and babeltrace executes successfully.
+
+SH_TAP=1
+
+if [ "x${BT_TESTS_SRCDIR:-}" != "x" ]; then
+	UTILSSH="$BT_TESTS_SRCDIR/utils/utils.sh"
+else
+	UTILSSH="$(dirname "$0")/../../utils/utils.sh"
+fi
+
+# shellcheck source=../../../utils/utils.sh
+source "$UTILSSH"
+
+traces_dir="${BT_CTF_TRACES_PATH}/deterministic-ordering"
+trace_a_corrupted="${traces_dir}/a-corrupted"
+trace_b_not_corrupted="${traces_dir}/b-not-corrupted"
+trace_c_corrupted="${traces_dir}/c-corrupted"
+
+if [ "$BT_OS_TYPE" = "mingw" ]; then
+	# The MSYS2 shell makes a mess trying to convert the Unix-like paths
+	# to Windows-like paths, so just disable the automatic conversion and
+	# do it by hand.
+	export MSYS2_ARG_CONV_EXCL="*"
+	trace_a_corrupted=$(cygpath -m "${trace_a_corrupted}")
+	trace_b_not_corrupted=$(cygpath -m "${trace_b_not_corrupted}")
+	trace_c_corrupted=$(cygpath -m "${trace_c_corrupted}")
+fi
+
+stdout_file=$(mktemp -t test_deterministic_ordering_stdout.XXXXXX)
+stderr_file=$(mktemp -t test_deterministic_ordering_stderr.XXXXXX)
+
+expect_failure() {
+	local test_name
+	local inputs
+
+	test_name="$1"
+	inputs="$2"
+
+	bt_cli "${stdout_file}" "${stderr_file}" \
+		-c src.ctf.fs -p "inputs=[${inputs}]"
+	isnt 0 "$?" "${test_name}: exit status is not 0"
+
+	grep --silent "^ERROR: " "${stderr_file}"
+	ok "$?" "${test_name}: error stack is produced"
+
+	grep --silent "No event class with ID of event class ID to use in stream class" "${stderr_file}"
+	ok "$?" "${test_name}: expected error message is present"
+}
+
+expect_success() {
+	local test_name
+	local inputs
+
+	test_name="$1"
+	inputs="$2"
+
+	bt_cli "${stdout_file}" "${stderr_file}" \
+		-c src.ctf.fs -p "inputs=[${inputs}]" \
+		-c sink.text.details -p 'with-trace-name=no,with-stream-name=no,with-metadata=no,compact=yes'
+	ok "$?" "${test_name}: exit status is 0"
+
+	bt_diff "${traces_dir}/b-c.expect" "${stdout_file}"
+	ok "$?" "${test_name}: expected output is produced"
+}
+
+plan_tests 10
+
+# Trace with corrupted packet comes first lexicographically, expect a failure.
+
+expect_failure "ab" "\"${trace_a_corrupted}\",\"${trace_b_not_corrupted}\""
+expect_failure "ba" "\"${trace_b_not_corrupted}\",\"${trace_a_corrupted}\""
+
+# Trace with non-corrupted packet comes first lexicographically, expect a success.
+
+expect_success "bc" "\"${trace_b_not_corrupted}\",\"${trace_c_corrupted}\""
+expect_success "cb" "\"${trace_c_corrupted}\",\"${trace_b_not_corrupted}\""
+
+rm -f "${stdout_file}" "${stderr_file}"