Repository navigation
Stamp one sequence number per buffer-aware publication - #1079
Draft
YuanYuYuan wants to merge 4 commits into
Draft
YuanYuYuan wants to merge 4 commits into
YuanYuYuan wants to merge 4 commits into
Conversation
publish_buffer_aware() took the next sequence number for every endpoint copy. A subscriber only sees its own copy, so with two or more endpoints it saw a gap on every message and reported lost messages that were delivered. Take one number per publication and stamp every copy with it. Signed-off-by: yuanyuyuan <az6980522@gmail.com>
Declare liveliness tokens that announce a backend named "fake", so that the publisher creates a per-subscriber endpoint for each of them without a GPU. Plain Zenoh subscribers record the attachment of every copy of a publication. Check that all copies of one publication carry the same sequence number and that consecutive publications advance it by one. Signed-off-by: yuanyuyuan <az6980522@gmail.com>
2 tasks
YuanYuYuan
changed the title
The comment states what the code guarantees and no longer describes the defect. The test parses liveliness tokens by segment instead of by substring.
mini-1235
reviewed
Oct 26, 2026
mini-1235
left a comment
mini-1235
left a comment
Contributor
There was a problem hiding this comment.
Hi, while reviewing the same function, I noticed a minor issue: publish_buffer_aware never emits the rmw_publish tracepoint, unlike the normal publish path.
Since this PR already touches the same function and the issue is minor, I thought it would be easier to leave a comment here rather than open a separate PR. I also left a question inline.
Comment on lines
+362
to
+363
| auto attachment_data = rmw_zenoh_cpp::AttachmentData( | ||
| publication_sequence_number(), rmw_zenoh_cpp::get_system_time_in_ns(), entity_->copy_gid()); |
Contributor
There was a problem hiding this comment.
Would it make sense to call rmw_zenoh_cpp::get_system_time_in_ns() just once in this function and reuse the timestamp here and L450?
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
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.
Fixes #1077. Part of #1075.
Summary
publish_buffer_awarestamped one sequence number per copy. A subscriber sees only its own copy, so it counted the other copies' numbers as lost messages. This PR stamps one number per publication, shared by all copies.Changes
PublisherData::publish_buffer_awareallocates the number after serialization, just before the first put attempt.Before and after
Tests
Measured at
d5898ecin a ROS Rolling container, five trials each. The base run usesrmw_publisher_data.cppfromdda52d6under the same tests.EveryCopyOfAPublicationCarriesTheSameNumberReproduction.
The fixture uses synthetic liveliness tokens and raw Zenoh samples. It does not run
message_lostcallbacks and needs no GPU. ros2/rmw_zenoh#1077 reports 118 lost messages per subscriber on Lyrical 0.10.6. This run does not repeat that measurement.Breaking Changes
None. With three endpoints, attachment numbers advance by one instead of three.
Did you use Generative AI?
Yes. Claude Sonnet 5.5 (
claude-sonnet-5-5) via Claude Code was used to assist with the changes, the tests and the measurements in this PR.