not all topics are discovered at startup

[tonynajjar]

Description

** clear and concise description of the bug is **

When using ros2 bag record --all --no-discovery, not all the topics that are available (the ones present with ros2 topic list) are subscribed to. I could not see a clear difference between the topics subscribed to and the ones left out.
I found a workaround that could help you debug the issue, see it here: pixel-robotics@0c885b3

To Reproduce

** Steps to reproduce the behavior, e.g.

1. Have a system with many topics open
2. run ros2 bag record --all --no-discovery
3. See that not all the topics available at startup are subscribed to

System (please complete the following information)

• OS: Ubuntu Jammy
• ROS 2 Distro: Humble
• Version: release
• DDS: Cyclone

Additional context

** Add any other context about the problem here **

[MichaelOrlov]

@tonynajjar

2. run ros2 bag record --all --no-discovery
3. See that not all the topics available at startup are subscribed to

This is looks like a WAD (works as design) for systems with DDS middleware.
To mitigate this issue in tests we are using wait_for_matched() for pub/sub manager for reference here

rosbag2/rosbag2_test_common/include/rosbag2_test_common/subscription_manager.hpp

Lines 86 to 128 in 1e4efa4

	/// \brief Wait until publishers will be connected to the subscribers or timeout occur.
	/// \tparam Timeout Data type for timeout duration from std::chrono:: namespace
	/// \param publishers List of raw pointers to the publishers
	/// \param timeout Maximum time duration during which discovery should happen.
	/// \param n_subscribers_to_match Number of subscribers each publisher should have for match.
	/// \return true if publishers have specified number of subscribers, otherwise false.
	template<typename Timeout>
	bool spin_and_wait_for_matched(
	const std::vector<rclcpp::PublisherBase *> & publishers,
	Timeout timeout, size_t n_subscribers_to_match = 1)
	{
	// Sanity check that we have valid input
	for (const auto publisher_ptr : publishers) {
	if (publisher_ptr == nullptr) {
	throw std::invalid_argument("Null pointer in publisher list");
	}
	std::string topic_name{publisher_ptr->get_topic_name()};
	if (expected_topics_with_size_.find(topic_name) == expected_topics_with_size_.end()) {
	throw std::invalid_argument(
	"Publisher's topic name = `" + topic_name + "` not found in expected topics list");
	}
	}
	using clock = std::chrono::steady_clock;
	auto start = clock::now();
	rclcpp::executors::SingleThreadedExecutor exec;
	bool matched = false;
	while (!matched && ((clock::now() - start) < timeout)) {
	exec.spin_node_some(subscriber_node_);
	matched = true;
	for (const auto publisher_ptr : publishers) {
	if (publisher_ptr->get_subscription_count() +
	publisher_ptr->get_intra_process_subscription_count() < n_subscribers_to_match)
	{
	matched = false;
	break;
	}
	}
	}
	return matched;
	}

In case of general rosbag2 recording with --all and --no-discovery options there are no way to determine which topics and how many publishers per topic to expect.

To avoid such issue I would recommend to use rosbag2 recorder with enabled discovery.

[tonynajjar]

Thank you for your answer. The reason why I want to avoid enabling discovery is because it uses a lot of CPU! (this can be yet another issue).
If this is WAD, then this feature request could work around the issue: implement a discovery timeout argument (turn off auto-discovery after elapsed duration)

[MichaelOrlov]

I don't think that it is a good idea to add a random delay in the code to workaround a problem.
If discovery taking a lot CPU is a root cause of the issue let's try to figure out how we can solve it.
Have you tried to use --polling-interval CLI option?

'-p', '--polling-interval', type=int, default=100,
            help='time in ms to wait between querying available topics for recording. '
                  'It has no effect if --no-discovery is enabled.'

Default value is 100 milliseconds.
Or explicitly specify topics which you need to record?
We automatically stopping discovery if all explicitly specified topics found.

[tonynajjar]

I don't think that it is a good idea to add a random delay in the code to workaround a problem.

It's not really a random delay, it's a fixed discovery timeout.

Have you tried to use --polling-interval CLI option?

I did try to to change the polling interval, it reduced the CPU usage by a little bit but it was still high.

Or explicitly specify topics which you need to record? We automatically stopping discovery if all explicitly specified topics found.

Sure that works but beats the purpose: I need to use the "--all" argument

We implemented a discovery timeout in our fork and that works well enough for now, but it would be nice if you could try to reproduce both bugs (high CPU usage and original reported bug) and validate it so we can be on the same page that this is a bug and not just some local issue.

[MichaelOrlov]

As a workaround instead of using ambient delay I would suggest to add service call to be able to stop and probably start discovery during recording. Similar to how pause/resume was implemented.

[emersonknapp]

So, you're saying that even with polling interval set to, for example, 1 second, it's still consuming a significant amount of CPU? What amount extra? 1%, 30%? It seems like doing a check like this once per second, even, should be relatively cheap - and I really feel like the best solution would probably be to try and fix any serious performance problems in it.

Additionally, does ros2 topic list give you all the topics you expect even on the first time running on the system? ros2 topic runs a daemon node process in the background that allows for "offline" discovery. If you do ros2 daemon stop and then do ros2 topic list, are you seeing all expected topics on that first list? This behavior should be approximately the same as the rosbag2 recording discovery. If it isn't then maybe there's something we need to address there to get these behaviors closer in sync.

[MichaelOrlov]

• Closing as stale issue.
• There are no response from author and issue was classified as works as design behavior.

[doisyg]

Seeing the same issue (CPU) which got much worse with Iron. On a system with multiple topics, actions and services (typically a mobile robot with the nav2 stack), with discovery enabled with see 20% CPU usage on a very powerful machine (i7 13th gen). The overhead was much lower in Humble.
Implementing a discovery timeout seems like a nice solution. @tonynajjar, do you have your fork around ? Would you be willing to PR it ?