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

Liveliness local readers #378

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

dpotman
Copy link
Contributor

@dpotman dpotman commented Jan 28, 2020

This PR adds support for liveliness QoS when using local readers. The implementation for (liveliness) expiration of writers used here is similar to that used with proxy writers, and it also supports the three liveliness kinds (1) automatic, which is trivial when using a local reader and writer, (2) manual-by-participant and (3) manual-by-topic.

In addition, these changes and fixes are included:

  • Fixed a bug in heartbeat handling in the reader: for manual-by-participant writers the lease was not updated on reception of a heartbeat message with liveliness flag set. This is fixed and a test-case is added.
  • Include the liveliness flag in a heartbeat message to the trace
  • Trace all lease renewals, including liveliness leases
  • Replaced liveliness changed state 'twitch' by 2 subsequent calls to the status callback
  • Added a test for liveliness duration 0 and 1ns (for both local and remote readers)

This commit adds support for liveliness QoS when using local readers.
The implementation for (liveliness) expiration of writers used here is
similar to that used with proxy writers, and it also supports the three
liveliness kinds (1) automatic, which is trivial when using a local
reader and writer, (2) manual-by-participant and (3) manual-by-topic.

In addition, these changes and fixes are included in this commit:
- Fixed a bug in heartbeat handling in the reader: for manual-by-
participant writers the lease was not updated on reception of a
heartbeat message with liveliness flag set. This is fixed and a
test-case is added.
- Include the liveliness flag in a heartbeat message to the trace
- Trace all lease renewals, including liveliness leases
- Replaced liveliness changed state 'twitch' by 2 subsequent calls
to the status callback
- Added a test for liveliness duration 0 and 1ns (for both local
and remote readers)

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
@eboasson
Copy link
Contributor

Thanks @dennis-adlink, there's really only one comment I have and that is that the need for a bit of refactoring to reduce the code duplication for local vs. proxy entities is really rather obvious. Which is not to say that I disagree with your choice here — indeed, I might well have made the same choice 😄 .

There have been a few maven problems in the builds and a switchover from travis-ci.org to travis-ci.com screwed up my ability to restart them. That should be over now, but I need to trigger a new build just to be sure — so I'll squash them by hand and do a force-push. But that's just administrative hassle after having decided to merge it.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
@eboasson eboasson merged commit 02c2753 into eclipse-cyclonedds:master Jan 31, 2020
@dpotman dpotman deleted the liveliness_local branch March 16, 2020 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants