Repository navigation
Do not drop pending SEDP samples when a new participant is discovered - #447
Open
johannesschrimpf wants to merge 1 commit into
Open
johannesschrimpf wants to merge 1 commit into
johannesschrimpf wants to merge 1 commit into
Conversation
When Discovery sees a participant for the first time, it calls
sedp_receive_subscription() and sedp_receive_publication() with that
participant's GUID prefix ("rediscovery"). Both read their built-in
reader with into_iterator(), which is take_bare(usize::MAX, not_read):
it takes every unread sample out of the DataReader. The result was then
filtered by the prefix, so the unread samples of every other participant
were removed without being processed. They never reached the DiscoveryDB
and no ReaderUpdated/WriterUpdated notification was sent, so no local
endpoint was matched with them.
Reliable retransmission does not recover the loss. SEDP is reliable, and
the RTPS Reader records reception independently of Discovery: its later
ACKNACKs report the sample as received, a resent DATA is ignored as a
duplicate, and the DataReader's per-writer read pointer (last_read_sn)
is past it. A fresh announcement of the endpoint with a new sequence
number (QoS change, endpoint recreated, participant restart) restores
discovery, but without one the failure can persist for the remaining
lifetime of the processes. Any unread SEDP sample available to these
readers when the participant-specific handler drains them is discarded
if its prefix differs from the requested participant's, so it happens
when participants start while others are still exchanging endpoints.
Process every taken sample, as the read_history == None path does. The
prefix now only decides the "Rediscovered reader" info log, and a debug
log reports each pending sample of another participant that rediscovery
processes. The secure variants carried the same filter but are only
called with None; drop the filter there too.
Add a regression test: it puts a reader and a writer of a third
participant, and the dispose of an earlier reader and writer of it,
into an observer's built-in SEDP reader caches without waking its
Discovery, then creates a new participant. It asserts that the
observer's DiscoveryDB then has the new reader and writer and no longer
has the disposed ones, that the observer's local DataWriter and
DataReader on that topic report PublicationMatched/SubscriptionMatched
for the new endpoints and an unmatch for the disposed ones, and that the
observer reports ReaderLost/WriterLost for the disposed ones. Without
the fix all of these fail (10 of 10 runs); with it all pass (10 of 10).
Validation on macOS:
- cargo test -- --test-threads=1, with and without --features security
- cargo clippy --all-targets, with and without --features security
- cargo +nightly fmt --check
Assisted-by: Claude Fable 5.1
Assisted-by: OpenAI Codex (gpt-6-astra)
This was referenced Oct 12, 2026
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
1 participant
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.
Summary
When RustDDS discovers a new participant, it re-reads its discovery readers
to pick up that participant's endpoints. That read takes every unread
endpoint announcement out of the reader and then keeps only the new
participant's. Announcements from other participants that happened to be
waiting are thrown away unprocessed. Reliable retransmission does not
recover these discarded samples, so an affected endpoint can remain
undiscovered until a fresh announcement is processed.
This change processes every taken announcement. A regression test covers
discovery, matching and disposal.
Mechanism
At c076919,
process_discovered_participant_data(
src/discovery/discovery.rs:1080-1087) callssedp_receive_subscriptionand
sedp_receive_publicationwith the new participant's GUID prefix. Bothfunctions (lines 1159 and 1229) read with
into_iterator(), which istake_bare(usize::MAX, ReadCondition::not_read())(
src/dds/with_key/datareader.rs:496-504): a take, not a read. The resultwas then filtered by the prefix (lines 1164-1172 and 1238-1246), and
everything filtered out was gone. Any unread SEDP sample available to these
readers when the participant-specific handler drains them is discarded if
its prefix differs from the requested participant's.
Why reliable retransmission does not recover the loss: the RTPS reader
records reception on its own, before Discovery looks at the sample. Later
ACKNACKs therefore report the sample as received, a resent copy would be
ignored as a duplicate, and the DataReader's per-writer read pointer
(
last_read_sn,src/dds/with_key/simpledatareader.rs:380-385) is alreadypast it. DDSI-RTPS 2.5 §8.5.4.2 makes SEDP reliable and §8.4.2.3 forbids
negatively acknowledging a sample after positively acknowledging it, so the
protocol cannot repair a loss that happens inside discovery processing
(https://www.omg.org/spec/DDSI-RTPS/2.5/PDF).
The endpoint stays unknown until it is announced again with a new sequence
number: after a QoS change, an endpoint recreation or a participant restart.
RustDDS stopped republishing endpoint data periodically in November 2023,
so without such an announcement the failure can persist for the remaining
lifetime of the processes.
What is affected
Any endpoint whose announcement is waiting unread in the discovery reader at
the moment another participant appears. Dispose announcements are lost the
same way, so a ReaderLost or WriterLost can go missing too. The window opens
whenever participants start while others are still exchanging endpoints.
History
non-destructive
readof the history. The prefix filter was harmless.into_iterator(), a take.The filter has discarded samples since then.
How it was found
On a robot with several ros2-client nodes on one host, one node's writer
never matched another node's reader for the whole run, while the reverse
direction and every other topic worked, and other participants were
starting at the time. I have not shown that this bug caused that case; the
robot's logs do not reach discovery level. Looking for a cause in the
discovery code led to the filter above, and the reproduction below shows
the bug directly.
Reproduction
A harness starts two nodes in separate processes, one publishing on a topic
and one subscribing to it, while four short-lived participants start and
stop around them. A round fails when the subscriber receives no sample
within 10 s. Match status is read from the children's "Matched new remote
reader/writer" log lines.
Baseline c076919, with a
warn!added for every sample the filterdiscards:
bystanders instead): the filter discarded samples in 156 rounds, 1723 in
total, mostly
ros_discovery_infoand parameter-service endpoints,including dispose samples.
participant discarding exactly the watched endpoint's announcement while
rediscovering a short-lived participant; that participant's log has no
match for the endpoint while the other side matched. No round that
received samples had such a discard.
endpoint. Their logs showed stalled or unusually slow SEDP delivery to
multiple peers. This change does not establish a fix for those failures.
With this change, same variants and round counts: 240 of 240 rounds received
samples. These runs enabled additional discovery debug logging, so they are
not a timing-matched comparison with the baseline. The new debug line
recorded 10,396 samples processed during rediscovery that the old filter
would have discarded. Six were announcements of the watched endpoints on
the receiving side; each was followed by a match within 3-11 ms.
The change
Every taken sample is processed, exactly as the
read_history == Nonepathalready does. The prefix now only selects the existing "Rediscovered
reader" info log, and a debug log reports each pending sample of another
participant that rediscovery processes.
secure_sedp_receive_subscriptionand
secure_sedp_receive_publicationhad the same filter but are onlycalled with
None, so the filter is removed there as well.sedp_receive_topic_dataalready ignored the prefix.Test
The regression test (
rediscovery_keeps_pending_sedp_samples_of_other_participantsin the tests module of discovery.rs) places pending endpoint announcements
and disposals from another participant in an observer's SEDP caches without
waking Discovery, then introduces a new participant. It checks DiscoveryDB
contents, local endpoint matching and unmatching
(PublicationMatched/SubscriptionMatched on the observer's own DataWriter and
DataReader), and ReaderLost/WriterLost notifications. Without the fix, it
failed in 10 of 10 runs; with the fix, it passed in 10 of 10 runs. Domain 47
reduces interference from other tests in this suite.
Validation (macOS)
cargo test -- --test-threads=1, with and without--features securitycargo clippy --all-targets, with and without--features securitycargo +nightly fmt --checkDeveloped with assistance from Claude Fable 5.1 and OpenAI Codex (gpt-6-astra).