storage: sort InternalReplicas before comparing#38593
storage: sort InternalReplicas before comparing#38593craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
pkg/storage/replica_command.go
Outdated
| return nil, errors.Wrap(err, "decoding current range descriptor value") | ||
| } | ||
| } | ||
| // TODO(jeffreyxiao): This is hacky fix to ensure that we don't fail the |
There was a problem hiding this comment.
Either This is a hacky fix is to ensure or This hacky fix is to ensure?
| } | ||
| // TODO(jeffreyxiao): This is hacky fix to ensure that we don't fail the | ||
| // conditional get because of the ordering of InternalReplicas. Calling | ||
| // Replicas() will sort the list of InternalReplicas as a side-effect. The |
There was a problem hiding this comment.
Can you add some more details about when and why this sorting became necessary?
a7c860d to
2ed5d15
Compare
jeffrey-xiao
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @nvanbenschoten)
pkg/storage/replica_command.go, line 1258 at r1 (raw file):
Previously, ajwerner wrote…
Either
This is a hacky fix is to ensureorThis hacky fix is to ensure?
Done.
pkg/storage/replica_command.go, line 1260 at r1 (raw file):
Previously, ajwerner wrote…
Can you add some more details about when and why this sorting became necessary?
Done.
nvb
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/storage/replica_command.go, line 1269 at r2 (raw file):
// ReplicaDescriptors.Learners(). existingDesc.Replicas() expectation.Replicas()
existingDesc.Replicas() // for sorting side-effect
expectation.Replicas() // for sorting side-effect
Fixes an issue where the in-memory copy of RangeDescriptor is out-of-sync with the copy in kv. Release note: None
2ed5d15 to
b44a71d
Compare
|
TFTRs! bors r+ |
38593: storage: sort InternalReplicas before comparing r=jeffrey-xiao a=jeffrey-xiao Fixes an issue where the in-memory copy of RangeDescriptor is out-of-sync with the copy in kv. Should resolve the failure in this #36983 (comment) The import step of `tpcc/mixed-headroom/n5cpu16` passes on `2811e0f` + #38465 + this PR. cc @asubiotto Co-authored-by: Jeffrey Xiao <jeffrey.xiao1998@gmail.com>
Build succeeded |
Fixes an issue where the in-memory copy of RangeDescriptor is out-of-sync with the copy in kv.
Should resolve the failure in this #36983 (comment)
The import step of
tpcc/mixed-headroom/n5cpu16passes on2811e0f+ #38465 + this PR.cc @asubiotto