cockroachdb / cockroachdb/cockroach
mma: handle decommissioned stores
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
When a store decommissions, MMA never removes its entry from clusterState.stores, nodeState.stores, or constraintMatcher.stores/allStores. The decommissioned store's last-reported Load and Capacity remain frozen in its storeState indefinitely (no further StoreLoadMsg ever updates it).
These stale values are then averaged into the cluster-wide mean used by the MMA shedding decision in (*rebalanceEnv).rebalanceStores (cluster_state_rebalance_stores.go:220):
`clusterMeans, ok := re.meansMemo.getMeans(nil)`
```
for _, ss := range re.stores {
sls := re.meansMemo.getStoreLoadSummary(ctx, clusterMeans, storeID, ss.loadSeqNum)
if sls.sls >= overloadSlow { /* add to shedding list */ }
}
```
There is no disposition filter between getMeans(nil) and the per-store classification at line 246. The classifier compares each live store against a mean that includes ghost load/capacity, so it can mis-classify live stores as over- or
under-loaded depending on what the ghost was doing when it left.
**Why the disposition gate doesn't help here: dispositions are checked in two places:**
- Target evaluation (allocator_state.go:962-1002) — retainReadyReplicaTargetStoresOnly drops refusing stores, and the mean is recomputed over the filtered set. Safe.
- Shedding-from check (comment at cluster_state_rebalance_stores.go:230-233) — we don't try to shed from a fdDrain / fdDead / storeMembershipRemoving store.
Neither path filters the cluster-wide mean itself. The shedding list is filtered downstream by disposition, but only after the polluted mean has already biased who got onto the list.
Append-only maps
- clusterState.stores: only written by insert; no delete anywhere on the top-level map.
- nodeState.stores: only appended to at cluster_state.go:2361.
- constraintMatcher.stores / allStores: only inserted at constraint_matcher.go:66. The header comment at constraint_matcher.go:18 references a cm.removeStore(...) API that was never implemented.
**Impact**
- Correctness: the cluster mean used to classify overload is biased by stale data from gone stores. The magnitude depends on cluster size and the ghost's frozen values, but the invariant "the mean reflects live cluster state" is silently
violated.
- Surfaced by: an observability metric introduced in #1009 (mma.store.cpu.immovable) uses len(ns.stores) as a divisor and becomes permanently wrong after a decommission until process restart.
**Fix**
Implement the missing removal path: when a store is decommissioned/removed, delete it from clusterState.stores, the relevant nodeState.stores, and constraintMatcher.stores/allStores. Wire this from the same path that flips the disposition to refusing.
Jira issue: CRDB-65377
Contributor guide
Research direction
Trace the disposition-to-refusing path and the store insertions in cluster_state.go, cluster_state_rebalance_stores.go, allocator_state.go, and constraint_matcher.go. Verify how clusterState.stores, nodeState.stores, and constraintMatcher.stores/allStores are populated before deciding where removal must be wired. Done means decommissioned stores leave all named collections and no longer affect MMA means or the immovable-store metric.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100