cockroachdb / cockroachdb/cockroach
kvserver: range descriptor passed to allocator should be used when calling change replicas
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
The range descriptor passed into `ChangeReplicas` is compared against the current descriptor within the `execChangeReplicasTxn`. If the descriptors are different, the replication change fails with a descriptor changed error, which is desirable given the ChangeReplicas was issued assuming a different set of replicas.
Currently, the replicate queue does not use the same descriptor from planning as it does for calling `ChangeReplicas`:
https://github.com/cockroachdb/cockroach/blob/1e8a4642d8fee46512f31514a53c6f22106c2b1b/pkg/kv/kvserver/replicate_queue.go#L814-L822
The store rebalancer has a slightly different variant of the same problem, because it calls `RelocateRange` instead of `ChangeReplicas` directly, so no descriptor is given:
https://github.com/cockroachdb/cockroach/blob/1e8a4642d8fee46512f31514a53c6f22106c2b1b/pkg/kv/kvserver/store_rebalancer.go#L731-L737
Note that validation does exist within `ChangeReplicas`, so that if the current descriptor is incompatible with the planned change, the change will fail:
https://github.com/cockroachdb/cockroach/blob/1e8a4642d8fee46512f31514a53c6f22106c2b1b/pkg/kv/kvserver/replica_command.go#L1831-L1849
This however doesn't check whether the range would become underreplicated or overreplicated from the proposed change.
Jira issue: CRDB-51105
Epic: CRDB-56265
Contributor guide
Assessment
This issue has not been assessed yet.