-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Adds support for fine-grained watches in blocking queries. #2671
Merged
Conversation
This file contains 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
kyhavlov
approved these changes
Jan 24, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! 👍
We can't actually return a fine-grained index from these tables unless support is added for tombstones. Otherwise, the index could slip backwards as things are deleted.
We always did an update before which caused excessive watch churn, even with our new fine-grained queries. This does a diff any only updates the node and service records if something actually changed.
Previously the blocking functions all closed over the state store from their first query, with would not have worked properly when a restore occurred. This makes sure they get a frest state store pointer each time, and that pointer is synchronized with the abandon watch.
slackpad
force-pushed
the
f-fine-watch
branch
from
January 25, 2017 17:58
8dde72c
to
b787aa1
Compare
Amazing, good job @slackpad ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This uses the newly-added watch support in go-memdb (via go-immutable-radix) to add fine-grained watches in blocking queries. This let us remove a bunch of hand-rolled code in the state store, and it means that when service X changes, it doesn't wake up callers blocking on service Y (fixes #1244). It also applies to all other areas of the state store.
In order to make this work, we also avoid no-op updates to node and service structures when only checks change. This helps prevent index churn on those upper structures and avoids extra unblocks.
The change in indexing did result in a slight change for KV watches, where there might be more spurious wakeups while waiting for a key to be created. This is the cheapest thing we can serve, so for now we will leave the code simple (it's at least a prefix watch), but we may want to consider a special case to make that more efficient if folks notice any performance impact from that.