kvserver: underreplicated ranges block lease transfers

[erikgrinaker]

See the lease preference setup in #106100 (comment), where we have 5 nodes with RF=5 across 3 racks, with lease preferences set to rack=0 (2 nodes). When we kill a node with rack=0, leases are randomly scattered across all surviving replicas. The replicate queue will (slowly) begin to move leases back to the preferred region.

However, as soon as the killed node is marked as dead, the replicate queue stops moving leases to the preferred region. It's erroring out on upreplication, since there are no stores that can take the replica, but this appears to also short-circuit the lease transfers, so the leases are now permanently stuck in a non-preferred region.

I230704 09:57:11.628687 181045 13@kv/kvserver/replicate_queue.go:753 ⋮ [T1,n2,replicate,s2,r595/4:‹/Table/106/1/9{18651…-20494…}›] 41934  error processing replica: ‹0 of 4 live stores are able to take a new replica for the range (4 already have a voter, 0 already have a non-voter); replicas must match constraints [{+rack=0:2} {+rack=1:2} {+rack=2:1}]; voting replicas must match voter_constraints []›

Jira issue: CRDB-29402

[kvoli]

This behavior regressed in 23.1 onwards with f2c3e12. Didn't raise any alarms because #88769 provided the only test coverage, and was skipped.

Lease preference satisfaction needs better test coverage and observability.

A lease transfer is not considered by the replicate queue (upreplication, removal, etc) where the leaseholder isn't removed. This is because the queue now performs at most one operation before returning from process().

This state persists until the upreplication the action doesn't error. If upreplication never occurs the action always errors¹, the preference won't be conformed to without some intervention (per-range) - ignoring load lease transfers

f2c3e12 also affected responsiveness when higher priority actions exist - however, it didn't change the duration required to satisfy all preferences in the worst case or change this behavior.

A fix could add back create an optional secondary lease transfer, regardless of the error value during process(). This isn't desirable, at least not without codifying it as an explicit part of an allocation op: for structure and compatibility with simulation (with what it can support).

Edit after some git history digging.

Footnotes

1. No valid upreplication targets is the example in the first comment. ↩

[kvoli]

I tested out the repro on v23.1.5 v22.2.11 and v22.1.21. All the prior versions run into this behavior. This is something we can, and should improve, however I'd caution against labelling it a bug. The intended behavior is to prioritize replication, over lease preference conformance. Perhaps we could be more explicit in our docs on lease preferences.

This behavior regressed in 23.1 onwards [...]

There was no regression. Non-snapshot errors on any process() action have always led to an early return. If the action always errors for a replica, such as ReplaceDeadVoter where there are no suitable targets, then we aren't guaranteed to consider a lease transfer.

In cases where the first process()¹ didn't error, we were previously (before #94023) more responsive to satisfying some, but not every range's lease preferences. It didn't do much for the unhappy case, though, where lease preference conformance will get stuck.

The situation isn't that hard to get out of currently, it just requires unblocking the cause of the errors; which are usually lack of available targets, e.g., the dead node was the only valid replica target for some (voter)constraint disjunction.

Footnotes

1. The first process() with non-conforming lease preferences. ↩

[kvoli]

This is now resolved via the lease queue #119155.