Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign in cache consumer and circular message cache to get rid from busy loop #941

Merged
merged 2 commits into from
Dec 16, 2021

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov MichaelOrlov commented Dec 14, 2021

Add wait_for_data() method to the message cache interface which will
block current thread and wait on condition variable until
notify_data_ready() will be called from producer thread.
Now logic will be more transparent in cache consumer IMO. i.e.

    message_cache_->wait_for_data();
    message_cache_->swap_buffers();
    auto consumer_buffer = message_cache_->get_consumer_buffer();
    consume_callback_(consumer_buffer->data());
    consumer_buffer->clear();
    message_cache_->release_consumer_buffer();

Also with this PR I think we will finally fix an issue related to the high probability to drop messages during the bag split.
Now there are no chance to drop messages unless there are no space in the cache any more. i.e. We can safely put new messages in primary buffer, while at the same time flushing messages to the storage from the secondary buffer.

@MichaelOrlov MichaelOrlov changed the title WIP Redesign in cache consumer and circular message cache to get rid from busy loop WIP: Redesign in cache consumer and circular message cache to get rid from busy loop Dec 14, 2021
@MichaelOrlov MichaelOrlov changed the title WIP: Redesign in cache consumer and circular message cache to get rid from busy loop Redesign in cache consumer and circular message cache to get rid from busy loop Dec 14, 2021
@MichaelOrlov MichaelOrlov marked this pull request as ready for review December 14, 2021 17:01
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner December 14, 2021 17:01
@MichaelOrlov MichaelOrlov requested review from emersonknapp, jhdcs and adamdbrw and removed request for a team December 14, 2021 17:01
@MichaelOrlov MichaelOrlov self-assigned this Dec 14, 2021
@MichaelOrlov MichaelOrlov requested review from emersonknapp and adamdbrw and removed request for emersonknapp and adamdbrw December 14, 2021 18:17
Add `wait_for_data()` method to the message cache interface which will
block current thread and wait on condition variable until
`notify_data_ready()` will be called from producer thread.

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the morlov/fix_for_busy_loop_in_snapshot_mode branch from 6d74777 to 5ef4140 Compare December 16, 2021 08:26
@MichaelOrlov
Copy link
Contributor Author

Running CI
CI_BRANCH_TO_TEST: morlov/fix_for_busy_loop_in_snapshot_mode
BUILD args: --packages-up-to rosbag2_cpp rosbag2_compression rosbag2_transport rosbag2_tests
TEST args: --packages-select rosbag2_cpp rosbag2_compression rosbag2_transport rosbag2_tests
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/9515

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@MichaelOrlov MichaelOrlov merged commit 2750c78 into master Dec 16, 2021
@delete-merged-branch delete-merged-branch bot deleted the morlov/fix_for_busy_loop_in_snapshot_mode branch December 16, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants