-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
kvserver: bring TestStoreRangeLease to a leader lease world #136229
Conversation
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.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani)
pkg/kv/kvserver/client_lease_test.go
line 64 at r1 (raw file):
// we can remove this. Leader leases require us to reject lease requests // on replicas that are not the leader, as otherwise we'll kvserver.RejectLeaseOnLeaderUnknown.Override(ctx, &st.SV, true)
What happens if this was removed?
This patch renames what was previously TestStoreRangeLease by adding a EpochLease suffix to it. Then, we rewrite TestStoreRangeLease to use leader leases instead. References cockroachdb#133763 Release note: None
2a4b6f3
to
28ba34c
Compare
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.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @iskettaneh)
pkg/kv/kvserver/client_lease_test.go
line 64 at r1 (raw file):
Previously, iskettaneh wrote…
What happens if this was removed?
It's not needed. Detritus from trying to stabilize this.
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.
Reviewable status: complete! 1 of 0 LGTMs obtained
Thanks! bors r+ |
This patch renames what was previously TestStoreRangeLease by adding a EpochLease suffix to it. Then, we rewrite TestStoreRangeLease to use leader leases instead.
References #133763
Release note: None