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

Periodic heartbeat also depending on ReaderProxy's low_mark [12642] #2253

Merged
merged 2 commits into from
Nov 19, 2021

Conversation

richiware
Copy link
Member

Using new flow controllers for low bandwidth communications I've realized periodic HEARTBEATS stop begin sent. This is caused my history's depth is low, and when the periodic heartbeat is triggered, all samples in the ReaderProxy are in UNSENT state.

This PR adds periodic HEARTBEAT also dependes in the low_mark. I f the low_mark is less than the minimun sequence number in the history, the HEARTBEAT is sent.

Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware richiware changed the title Periodic heartbeat also depending on ReaderProxy's low_mark Periodic heartbeat also depending on ReaderProxy's low_mark [12642] Oct 8, 2021
@MiguelCompany
Copy link
Member

What should happen if the writer's history is empty but the low mark of the reader proxy is below next_sequence_number() ?

@richiware
Copy link
Member Author

What should happen if the writer's history is empty but the low mark of the reader proxy is below next_sequence_number() ?

Technically the HEARTBEAT won't be sent. Practically (using DDS-API) the history never will be empty until the samples are acknowledge.

@MiguelCompany
Copy link
Member

history never will be empty

There's the case when you create a DataWriter, and then remove it. In that case, the EDP publications writer will have an empty history, but the min seq will be 2 instead of 1.

@MiguelCompany MiguelCompany self-assigned this Oct 14, 2021
src/cpp/rtps/writer/StatefulWriter.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/writer/StatefulWriter.cpp Outdated Show resolved Hide resolved
@MiguelCompany MiguelCompany added this to the v2.4.1 milestone Nov 2, 2021
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@MiguelCompany MiguelCompany removed their assignment Nov 17, 2021
@MiguelCompany
Copy link
Member

@richiprosima Please test discovery-server

@MiguelCompany MiguelCompany added the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Nov 19, 2021
@MiguelCompany MiguelCompany merged commit a82ee68 into master Nov 19, 2021
@MiguelCompany MiguelCompany deleted the bugfix/heartbeart-low-mark branch November 19, 2021 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants