From c488567db9b8c317fcafbbc4cc925308c562381a Mon Sep 17 00:00:00 2001 From: james-rms Date: Fri, 23 Dec 2022 06:26:27 +1100 Subject: [PATCH] rosbag2_storage: set MCAP as default plugin (#1160) switches the default storage plugin from `sqlite3` to `mcap`. ## Reason for change Benchmarks suggest that MCAP can support higher write throughput than the `sqlite3` plugin in its default configuration, while also bringing the following other benefits: * Append-only write behavior ensures that only the last few messages written can be corrupted in the case of a power outage or recorder crash. This is similar to the corruption guarantees offered by SQLite3 in [WAL mode](https://sqlite.org/wal.html), which is what is used in the `resilient` SQLite storage plugin preset. * Ability to enable chunk compression, which should be more space-efficient than message compression for bags with many small messages. This also enables reader to use the message index, unlike file-level compression. * Record message schemas to the bag by default, enabling applications outside the ROS 2 workspace to read message content. ## Benchmarks Read the benchmarking analysis that lead to this conclusion [here](https://mcap.dev/performance-comparison/rosbag2-plugin-comparison.html). ### `rosbag2_performance_benchmarking` There is a set of pre-existing benchmarks in the rosbag2 repo here: https://github.com/ros2/rosbag2/tree/rolling/rosbag2_performance/rosbag2_performance_benchmarking We've run these in an attempt to compare the performance of MCAP against SQLite on these metrics, but at this point the results appear to be pure noise. A full analysis of why will be published to mcap.dev shortly, but for now the news is that we have not succeeded in producing a statistically significant difference in results between MCAP and SQLite. ## TODO - [x] https://github.com/ros2/rosbag2/issues/1112 - [x] https://github.com/ros-tooling/rosbag2_storage_mcap/issues/61 - [x] https://github.com/ros-tooling/rosbag2_storage_mcap/issues/63 - [x] https://github.com/ros2/rosbag2/issues/1185 - [x] Parametrize `rosbag2_tests` --- ros2bag/package.xml | 2 -- ros2bag/test/test_record_qos_profiles.py | 18 ++++++++---------- rosbag2_cpp/package.xml | 3 --- rosbag2_py/package.xml | 2 -- .../src/rosbag2_storage/default_storage_id.cpp | 2 +- rosbag2_storage_default_plugins/package.xml | 1 + rosbag2_tests/package.xml | 2 -- rosbag2_transport/package.xml | 1 - 8 files changed, 10 insertions(+), 21 deletions(-) diff --git a/ros2bag/package.xml b/ros2bag/package.xml index 6ad74f5b46..b2827e9567 100644 --- a/ros2bag/package.xml +++ b/ros2bag/package.xml @@ -24,8 +24,6 @@ launch_testing_ros python3-pytest rosbag2_storage_default_plugins - - rosbag2_storage_mcap rosbag2_test_common diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index 5b4918b7e3..3bfff87773 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -84,32 +84,30 @@ def test_qos_simple(self): output_path = Path(self.tmpdir.name) / 'ros2bag_test_basic' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] - expected_string_regex = re.compile( - r'\[rosbag2_storage]: Opened database .* for READ_WRITE') + expected_output = 'Listening for topics...' with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( - condition=lambda output: expected_string_regex.search(output) is not None, + condition=lambda output: expected_output in output, timeout=OUTPUT_WAIT_TIMEOUT) bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated - matches = expected_string_regex.search(bag_command.output) - assert matches, ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output) + matches = expected_output in bag_command.output + assert matches, ERROR_STRING_MSG.format(expected_output, bag_command.output) def test_incomplete_qos_profile(self): profile_path = PROFILE_PATH / 'incomplete_qos_profile.yaml' output_path = Path(self.tmpdir.name) / 'ros2bag_test_incomplete' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] - expected_string_regex = re.compile( - r'\[rosbag2_storage]: Opened database .* for READ_WRITE') + expected_output = 'Listening for topics...' with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( - condition=lambda output: expected_string_regex.search(output) is not None, + condition=lambda output: expected_output in output, timeout=OUTPUT_WAIT_TIMEOUT) bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated - matches = expected_string_regex.search(bag_command.output) - assert matches, ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output) + matches = expected_output in bag_command.output + assert matches, ERROR_STRING_MSG.format(expected_output, bag_command.output) def test_incomplete_qos_duration(self): profile_path = PROFILE_PATH / 'incomplete_qos_duration.yaml' diff --git a/rosbag2_cpp/package.xml b/rosbag2_cpp/package.xml index 9dfa3b2939..ca98185b28 100644 --- a/rosbag2_cpp/package.xml +++ b/rosbag2_cpp/package.xml @@ -27,9 +27,6 @@ shared_queues_vendor rosbag2_storage_default_plugins - - rosbag2_storage_mcap - ament_cmake_gmock ament_lint_auto ament_lint_common diff --git a/rosbag2_py/package.xml b/rosbag2_py/package.xml index 69640f5d92..92f8c2ad31 100644 --- a/rosbag2_py/package.xml +++ b/rosbag2_py/package.xml @@ -32,8 +32,6 @@ rcl_interfaces rclpy rosbag2_storage_default_plugins - - rosbag2_storage_mcap rosbag2_test_common rosidl_runtime_py std_msgs diff --git a/rosbag2_storage/src/rosbag2_storage/default_storage_id.cpp b/rosbag2_storage/src/rosbag2_storage/default_storage_id.cpp index 13487307bf..6bbadd81e9 100644 --- a/rosbag2_storage/src/rosbag2_storage/default_storage_id.cpp +++ b/rosbag2_storage/src/rosbag2_storage/default_storage_id.cpp @@ -18,7 +18,7 @@ namespace rosbag2_storage std::string get_default_storage_id() { - return "sqlite3"; + return "mcap"; } } // namespace rosbag2_storage diff --git a/rosbag2_storage_default_plugins/package.xml b/rosbag2_storage_default_plugins/package.xml index e6a5397906..1699e69f35 100644 --- a/rosbag2_storage_default_plugins/package.xml +++ b/rosbag2_storage_default_plugins/package.xml @@ -13,6 +13,7 @@ ament_cmake + rosbag2_storage_mcap rosbag2_storage_sqlite3 diff --git a/rosbag2_tests/package.xml b/rosbag2_tests/package.xml index e13a67f534..d5aff85c1e 100644 --- a/rosbag2_tests/package.xml +++ b/rosbag2_tests/package.xml @@ -25,8 +25,6 @@ rosbag2_compression_zstd rosbag2_cpp rosbag2_storage_default_plugins - - rosbag2_storage_mcap rosbag2_storage rosbag2_test_common std_msgs diff --git a/rosbag2_transport/package.xml b/rosbag2_transport/package.xml index 1e424501cc..f4075165c7 100644 --- a/rosbag2_transport/package.xml +++ b/rosbag2_transport/package.xml @@ -31,7 +31,6 @@ rosbag2_compression_zstd rosbag2_test_common rosbag2_storage_default_plugins - rosbag2_storage_mcap test_msgs