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

Allow participants to sync up with remote databases when discrepancies arise #145

Merged
merged 9 commits into from
Dec 23, 2021

Conversation

mxgrey
Copy link
Contributor

@mxgrey mxgrey commented Dec 1, 2021

There is a very small probability in the event of server restart race conditions that a participant may disagree with its remote database on details like its participant ID and description.

This PR allows participants to identify when these issues occur and work out a correction with the remote database.

We limit the rate at which these corrections can occur, because there is a risk of a pathological case where two participant instances believe they both own the same name. If that happens then they might viciously cycle against each other, each trying to "correct" the "bad" information that's being pushed by the other. In the worst case scenario that could be a very tight loop of hammering "corrective" messages at the schedule node. We limit the rate to 3 corrections per minute. Corrections happening at a higher rate than that should be brought to the attention of an operator.

To test this PR you can run four terminals with the following commands:

$  ros2 run rmf_traffic_ros2 changed_participant_schedule_node
$  ros2 run rmf_traffic_ros2 mock_repetitive_delay_participant
$ ros2 topic echo /rmf_traffic/itinerary_delay
$ ros2 topic echo /rmf_traffic/participants

Then watch as the participant makes corrections to the sabotage that the mocked up schedule node is causing. This scenario is also designed to demonstrate the rate limiting of the corrections.

This PR depends on

gbiggs and others added 3 commits September 30, 2021 16:47
Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

This is looks amazing! Thanks for the explanation and comments along the way. I just have a few nitpicks, but in general I'm seeing the intended behavior and it should be in good shape to be merged after the base branch is updated.

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #145 (fd2def0) into main (a88fd24) will decrease coverage by 0.30%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
- Coverage   21.52%   21.21%   -0.31%     
==========================================
  Files         422      856     +434     
  Lines       34378    69752   +35374     
  Branches    16260    32920   +16660     
==========================================
+ Hits         7401    14801    +7400     
- Misses      19043    39106   +20063     
- Partials     7934    15845    +7911     
Flag Coverage Δ
tests 21.21% <ø> (-0.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
..._adapter/src/rmf_fleet_adapter/ScheduleManager.cpp
...s2/src/rmf_traffic_ros2/schedule/internal_Node.hpp
...2/rmf_fleet_adapter/test/phases/test_GoToPlace.cpp
...fleet_adapter/src/rmf_fleet_adapter/estimation.cpp
...-4.1.0/Rx/v2/src/rxcpp/schedulers/rx-eventloop.hpp
...RxCpp-4.1.0/Rx/v2/src/rxcpp/operators/rx-merge.hpp
...f_fleet_adapter/agv/internal_RobotUpdateHandle.hpp
...rc/rmf_traffic_ros2/schedule/convert_Itinerary.cpp
...er/src/rmf_fleet_adapter/agv/FleetUpdateHandle.cpp
.../rmf_fleet_adapter/src/mock_traffic_light/main.cpp
... and 1268 more

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
…open-rmf/rmf_ros2 into gbiggs/add-participant-robustness
Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Tests and demos are working fine, the example you provided with is working as expected too. Thanks!

@aaronchongth aaronchongth merged commit ccf18ea into main Dec 23, 2021
@aaronchongth aaronchongth deleted the gbiggs/add-participant-robustness branch December 23, 2021 08:49
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.

None yet

3 participants