cockroachdb / cockroachdb/cockroach

kvclient: do not evict range descriptors on stale RangeKeyMismatchError

Open
#105,294 4 comments 0 reactions 1 assignee Assigned to @arulajmani View on GitHub
A-kv-transactions C-bug O-support P-3 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

When the `DistSender` receives a `RangeKeyMismatchErrors` it simply terminates its inner loop (`sendToReplicas`) and evicts the range descriptor. In doing so, it updates its range cache with the range descriptors in the error; see:

https://github.com/cockroachdb/cockroach/blob/5dd802945591fce48b1e357e4a9f91ec13f82057/pkg/kv/kvclient/kvcoord/dist_sender.go#L1895-L1908

Now, if the error is returned by a replica that's lagging (say it hasn't applied a split or a merge yet), we'll end up needlessly evicting the range descriptor. Worse yet, if on the subsequent retry the sender picks this replica[1] as the first one to route to again, we'll find ourselves in an infinite loop until this replica is caught up.

**Proposed solution**

We know when a `RangeKeyMismatchError` is originating from a replica that isn't caught up yet, by virtue of the descriptor generation in the first entry of its `Ranges` slice. Currently, `sendToReplicas` short circuits without peeking into this slice:

https://github.com/cockroachdb/cockroach/blob/5dd802945591fce48b1e357e4a9f91ec13f82057/pkg/kv/kvclient/kvcoord/dist_sender.go#L2533-L2536

Maybe it should. That is, it should only terminate its iteration if the `RangeKeyMismatch` error is coming from a replica that has a descriptor generation newer than what's in the range cache.

cc @kvoli @andrewbaptist

----

[1] This can happen if the sender believes this replica has the lease or if it has no lease information and the replica is closest to it.

Jira issue: CRDB-28967

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.