Make the player respect the original messages' order with the same timestamp #2172
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add insertion sequence number to the LockedPriorityQueue to ensure that it can provide stable runtime sort algorithm. Signed-off-by: Michael Orlov <morlovmr@gmail.com>
MichaelOrlov
changed the title
MichaelOrlov
force-pushed
the
morlov/respect-msg-order-in-piority-queue
branch
from
d91dd30 to
bf14ecd
Compare
MichaelOrlov
changed the title
Contributor
Author
|
cc: @DaleKoenig |
MichaelOrlov
added a commit
that referenced
this pull request
Sep 14, 2025
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
MichaelOrlov
added a commit
that referenced
this pull request
Sep 19, 2025
* Bugfix: Player can't play with read_ahead_queue_size equal 1 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add missing deserialize_test_message() from #2172 Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Address review comments Co-Authored by: Christophe Bedard <bedard.christophe@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
mergify bot
pushed a commit
that referenced
this pull request
Sep 19, 2025
* Bugfix: Player can't play with read_ahead_queue_size equal 1 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add missing deserialize_test_message() from #2172 Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Address review comments Co-Authored by: Christophe Bedard <bedard.christophe@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> (cherry picked from commit 951d175)
mergify bot
pushed a commit
that referenced
this pull request
Sep 19, 2025
* Bugfix: Player can't play with read_ahead_queue_size equal 1 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add missing deserialize_test_message() from #2172 Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Address review comments Co-Authored by: Christophe Bedard <bedard.christophe@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> (cherry picked from commit 951d175) # Conflicts: # rosbag2_transport/test/rosbag2_transport/rosbag2_transport_test_fixture.hpp
Contributor
Author
|
Pulls: #2172
|
christophebedard
approved these changes
Sep 22, 2025
Member
|
I found the job that was retriggered for ci_linux-aarch64, but the ci_windows job wasn't automatically retriggered, so I did it manually. |
MichaelOrlov
added a commit
that referenced
this pull request
Sep 23, 2025
…2179) * Bugfix: Player can't play with read_ahead_queue_size equal 1 * Add missing deserialize_test_message() from #2172 * Address review comments Co-Authored by: Christophe Bedard <bedard.christophe@gmail.com> --------- (cherry picked from commit 951d175) Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <morlovmr@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
MichaelOrlov
added a commit
that referenced
this pull request
Sep 23, 2025
…(backport #2174) (#2180) * Bugfix: Player can't play with read_ahead_queue_size equal 1 (#2174) * Bugfix: Player can't play with read_ahead_queue_size equal 1 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add missing deserialize_test_message() from #2172 Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Address review comments Co-Authored by: Christophe Bedard <bedard.christophe@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> (cherry picked from commit 951d175) # Conflicts: # rosbag2_transport/test/rosbag2_transport/rosbag2_transport_test_fixture.hpp * Address merge conflicts Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Adjust test for absence of the "play_options_.message_order" Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <morlovmr@gmail.com>
Contributor
Author
|
https://github.com/Mergifyio backport kilted jazzy |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Sep 23, 2025
…mestamp (#2172) * Make player respect original msgs order with the same timestamp - Add insertion sequence number to the LockedPriorityQueue to ensure that it can provide stable runtime sort algorithm. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Simplify comparator's API by hiding inner sequence number inside queue Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> (cherry picked from commit 6b462de)
mergify bot
pushed a commit
that referenced
this pull request
Sep 23, 2025
…mestamp (#2172) * Make player respect original msgs order with the same timestamp - Add insertion sequence number to the LockedPriorityQueue to ensure that it can provide stable runtime sort algorithm. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Simplify comparator's API by hiding inner sequence number inside queue Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> (cherry picked from commit 6b462de) # Conflicts: # rosbag2_transport/src/rosbag2_transport/player.cpp
MichaelOrlov
added a commit
that referenced
this pull request
Sep 24, 2025
…mestamp (#2172) (#2187) * Make player respect original msgs order with the same timestamp - Add insertion sequence number to the LockedPriorityQueue to ensure that it can provide stable runtime sort algorithm. * Simplify comparator's API by hiding inner sequence number inside queue --------- (cherry picked from commit 6b462de) Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <morlovmr@gmail.com>
MichaelOrlov
added a commit
that referenced
this pull request
Sep 26, 2025
… same timestamp (backport #2172) (#2188) * Make the player respect the original messages' order with the same timestamp (#2172) * Make player respect original msgs order with the same timestamp - Add insertion sequence number to the LockedPriorityQueue to ensure that it can provide stable runtime sort algorithm. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Simplify comparator's API by hiding inner sequence number inside queue Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> (cherry picked from commit 6b462de) # Conflicts: # rosbag2_transport/src/rosbag2_transport/player.cpp * Address merge conflicts and code difference Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <morlovmr@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Make the player respect the original messages' order with the same timestamp.
Note: The messages with the same
recv_timestampsare a common situation when doing a recording withsim_timeAdded insertion sequence number to the LockedPriorityQueue to ensure that it can provide a stable runtime sort algorithm.
Fixes Player does not preserve the order of messages with the same timestamp #2173
Is this user-facing behavior change?
Yes. Playback will respect the originally saved messages' order even if messages have the same timestamps.
However, playback from multiple bags can't preserve the original order of messages with the same timestamps in different bags because we don't know which one was first during recording, although the order is at least the same for each playback.
Did you use Generative AI?
Yes, I am using GitHub Copilot, GPT-4.1 in my workflow.
Additional Information
N/A