src.ctf.fs: sort inputs paths
authorSimon Marchi <simon.marchi@efficios.com>
Fri, 15 Nov 2019 22:20:49 +0000 (17:20 -0500)
committerSimon Marchi <simon.marchi@efficios.com>
Wed, 20 Nov 2019 22:45:43 +0000 (17:45 -0500)
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
tests/Makefile.am
tests/data/ctf-traces/deterministic-ordering/a-corrupted/dummystream [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/a-corrupted/metadata [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/b-c.expect [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/dummystream [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/b-not-corrupted/metadata [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/c-corrupted/dummystream [new file with mode: 0644]
tests/data/ctf-traces/deterministic-ordering/c-corrupted/metadata [new file with mode: 0644]
tests/plugins/src.ctf.fs/Makefile.am
tests/plugins/src.ctf.fs/test_deterministic_ordering [new file with mode: 0755]

index ceeb4bc268436141154e59b3bfe8b9502bf51a19..e8121140e8251967b967e33b5d57e0328274071b 100644 (file)
@@ -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;
 }
 
index 58937d759d081004ec66b8e71409e7f8b1046e5b..5213707d200f954f0640e32fa2a3e117cfcb98b9 100644 (file)
@@ -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 (file)
index 0000000..f0380a1
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 (file)
index 0000000..9008c15
--- /dev/null
@@ -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 (file)
index 0000000..c21129b
--- /dev/null
@@ -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 (file)
index 0000000..f69dc7e
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 (file)
index 0000000..9008c15
--- /dev/null
@@ -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 (file)
index 0000000..f0380a1
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 (file)
index 0000000..9008c15
--- /dev/null
@@ -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;
+};
index 7aacb3eba40bfda727adc61c95c5b52a7977bec9..1218f2cd1d030f978e41d8f75b2508202027510f 100644 (file)
@@ -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 (executable)
index 0000000..27d317e
--- /dev/null
@@ -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}"
This page took 0.031001 seconds and 4 git commands to generate.