From: Simon Marchi Date: Fri, 15 Nov 2019 22:20:49 +0000 (-0500) Subject: src.ctf.fs: sort inputs paths X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=7b69723d5301785f7ea9b7bfec0165b47a8cf0e5 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2396 --- diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index ceeb4bc2..e8121140 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/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 @@ error: 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; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 58937d75..5213707d 100644 --- a/tests/Makefile.am +++ b/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 diff --git a/tests/data/ctf-traces/deterministic-ordering/a-corrupted/dummystream b/tests/data/ctf-traces/deterministic-ordering/a-corrupted/dummystream new file mode 100644 index 00000000..f0380a19 Binary files /dev/null and b/tests/data/ctf-traces/deterministic-ordering/a-corrupted/dummystream differ diff --git a/tests/data/ctf-traces/deterministic-ordering/a-corrupted/metadata b/tests/data/ctf-traces/deterministic-ordering/a-corrupted/metadata new file mode 100644 index 00000000..9008c153 --- /dev/null +++ b/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; +}; diff --git a/tests/data/ctf-traces/deterministic-ordering/b-c.expect b/tests/data/ctf-traces/deterministic-ordering/b-c.expect new file mode 100644 index 00000000..c21129b2 --- /dev/null +++ b/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 diff --git a/tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/dummystream b/tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/dummystream new file mode 100644 index 00000000..f69dc7ee Binary files /dev/null and b/tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/dummystream differ diff --git a/tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/metadata b/tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/metadata new file mode 100644 index 00000000..9008c153 --- /dev/null +++ b/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; +}; diff --git a/tests/data/ctf-traces/deterministic-ordering/c-corrupted/dummystream b/tests/data/ctf-traces/deterministic-ordering/c-corrupted/dummystream new file mode 100644 index 00000000..f0380a19 Binary files /dev/null and b/tests/data/ctf-traces/deterministic-ordering/c-corrupted/dummystream differ diff --git a/tests/data/ctf-traces/deterministic-ordering/c-corrupted/metadata b/tests/data/ctf-traces/deterministic-ordering/c-corrupted/metadata new file mode 100644 index 00000000..9008c153 --- /dev/null +++ b/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; +}; diff --git a/tests/plugins/src.ctf.fs/Makefile.am b/tests/plugins/src.ctf.fs/Makefile.am index 7aacb3eb..1218f2cd 100644 --- a/tests/plugins/src.ctf.fs/Makefile.am +++ b/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 diff --git a/tests/plugins/src.ctf.fs/test_deterministic_ordering b/tests/plugins/src.ctf.fs/test_deterministic_ordering new file mode 100755 index 00000000..27d317ec --- /dev/null +++ b/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}"