kvserver: clean-up the interaction between the mergeQueue and AdminRelocateRange#75981
Conversation
|
This change is |
nvb
left a comment
There was a problem hiding this comment.
This works for v22.1 nodes, but does it work in a mixed-version cluster? Should we gate this logic on a cluster version?
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15)
bbfd569 to
b9c86b1
Compare
Ah, great catch. It's kind of unfortunate though. It seems fine to keep the logic that moves the leaseholder to the front, just with the new |
nvb
left a comment
There was a problem hiding this comment.
It seems fine to keep the logic that moves the leaseholder to the front, just with the new transferLeaseToFirstVoter set to false. This gets us the same thing as version gating it, right?
Good idea, I don't see any issues with this.
Reviewed all commit messages.
Reviewable status:
pkg/kv/kvserver/merge_queue.go, line 340 at r2 (raw file):
// AdminRelocateRange moves the lease to the first target in the list, so // sort the existing leaseholder there to leave it unchanged. // TODO(aayush): Remove this for 22.2.
nit: make it more clear what "this" is, along with why we're now able to make a change here.
b9c86b1 to
d56021b
Compare
…ateRange` This commit makes it such that the `mergeQueue` no longer has to take special care to avoid a lease transfer when calling into `AdminRelocateRange`. This was enabled by cockroachdb#75676. The preceding logic can be cleaned up in the 22.2 cycle. Release note: None
d56021b to
8d55acb
Compare
aayushshah15
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/kv/kvserver/merge_queue.go, line 340 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: make it more clear what "this" is, along with why we're now able to make a change here.
Done.
|
Build failed (retrying...): |
|
Build succeeded: |
This commit makes it such that the
mergeQueueno longer has to take specialcare to avoid a lease transfer when calling into
AdminRelocateRange. This wasenabled by #75676.
Release note: None