cockroachdb / cockroachdb/cockroach
kvserver: allocator and node vitality have conflicting suspect states
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
Prior to 23.2, the concept of a store (or node by extension) being `suspect` was only found in the store pool and allocator[^1].
In 23.2, `NodeVitality` was introduced, which also introduced the concept of `suspect`, however did not also introduce a `suspect` related state, reusing `NodeLivenessStatus_UNAVAILABLE`.
The storepool also now calls into `NodeVitality` to determine if a node is alive, dead, unavailable, decommissioning or unknown, where previously it called `NodeLiveness`.
When `NodeVitality` determines that a node should be considered `suspect` by having last received a store descriptor from that node more than the `gossip.StoreTTL` ago, in addition to another 30s (default `server.time_after_store_suspect` duration).
`NodeVitality` then returns `NodeLivenessStatus_UNAVAILABLE` from:
https://github.com/cockroachdb/cockroach/blob/a0f39e7ac9574756063bc90bba6bc532b45c33d4/pkg/kv/kvserver/liveness/livenesspb/liveness.go#L429-L429
In the store pool however, the suspect condition is based on the last time `NodeLivenessStatus_UNAVAILABLE` was returned. This implies that the suspect duration may be double counted[^2] in some instances, while also not correctly accounting for allocator storepool calls which wish to still include suspect nodes in consideration:
https://github.com/cockroachdb/cockroach/blob/a0f39e7ac9574756063bc90bba6bc532b45c33d4/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go#L996-L1003
Which is relied upon for correct `AllocatorAction` quorum/availability determination and `ChangeReplicasTxn`:
https://github.com/cockroachdb/cockroach/blob/a0f39e7ac9574756063bc90bba6bc532b45c33d4/pkg/kv/kvserver/replica_command.go#L1217-L1223
We should rectify these inconsistencies and fully assess the impact of them, not otherwise stated here.
[^1]: https://github.com/cockroachdb/cockroach/blob/7002082b08ad63d1c4f8596b1ece693361268c75/pkg/kv/kvserver/allocator/storepool/store_pool.go#L54-L74
[^2]: https://github.com/cockroachdb/cockroach/blob/a0f39e7ac9574756063bc90bba6bc532b45c33d4/pkg/kv/kvserver/allocator/storepool/store_pool.go#L189-L194
Jira issue: CRDB-43317
Epic CRDB-37526
Contributor guide
Assessment
This issue has not been assessed yet.