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

Do not update tracker clock with registry entry clocks #116

Merged
merged 3 commits into from
Dec 14, 2018

Conversation

tschmittni
Copy link
Contributor

This is a follow up of #85

I forgot to change the handle_topology_change method which is
as far as I can tell also the root cause for
#106

This is a follow up of bitwalker#85

I forgot to change the handle_topology_change method which is
as far as I can tell also the root cause for
bitwalker#106
@tschmittni
Copy link
Contributor Author

I managed to reproduce #106 and this is happening because the clock gets forked on every anti-entropy sync. I removed the fork and added some more debug output when clock-forks are happening which should only be when a new node joins a cluster.

@arjan
Copy link
Collaborator

arjan commented Nov 16, 2018

This is great! Do you think the situation you describe could be made reproducible in a testcase?

- Added test for inspecting the node state after sync to make sure
  that clocks are still correct.
- Fix sync test which was not testing concurrent clocks
@tschmittni
Copy link
Contributor Author

Added a test case

@beardedeagle
Copy link
Collaborator

This looks good so far, as I've been able to duplicate #106 myself I am going to pull this down and give it a spin.

@picaoao
Copy link
Contributor

picaoao commented Dec 3, 2018

Tests are failing ... Any news on this ? This seems like a pretty critical and important fix !

@tschmittni
Copy link
Contributor Author

Tests have been failing on the master branch before. Did this change cause any new failures?

@beardedeagle beardedeagle merged commit 55d7994 into bitwalker:master Dec 14, 2018
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.

4 participants