-
Notifications
You must be signed in to change notification settings - Fork 77
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
refactor: remove read state #857
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #857 +/- ##
==========================================
+ Coverage 75.55% 75.61% +0.06%
==========================================
Files 180 187 +7
Lines 26938 27618 +680
Branches 26938 27618 +680
==========================================
+ Hits 20353 20884 +531
- Misses 5366 5447 +81
- Partials 1219 1287 +68 ☔ View full report in Codecov by Sentry. |
refactor: disable fast path in etcd competible layer WIP: fix rebase kv server Signed-off-by: bsbds <[email protected]>
@bsbds Convert your pr to draft since CI failed |
Signed-off-by: bsbds <[email protected]>
Signed-off-by: bsbds <[email protected]>
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.
Please leave an issue to track this problem: when a network partition occurs, the leader in the smaller partition may not be aware that a new leader has already been elected in the cluster. If the original leader continues to serve read-only requests to clients during this time, it could result in stale reads.
Tracked in #870 |
refactor: disable fast path in etcd competible layer WIP: fix rebase kv server Signed-off-by: bsbds <[email protected]>
Depends-On: #851 #852
Closes: #782, #501, #497, as we don't rely on this mechanism any more
Removes the current read state mechanism, and sets all proxied etcd commands to use slow path.
The read state requires the leader to query it's
UncommittedPool
. However, for every query, we must lock it in another tokio worker thread. Lock in multiple threads could reduce general performance, I think it's best to remove the current implementation and reimplement it in the future.All etcd commands need to use slow path because we need 2RTTs to acquire a revision.
Please briefly answer these questions:
what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)
what changes does this pull request make?
are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)