cockroachdb / cockroachdb/cockroach
raft: stepped-down leaders re-read the committed term from storage every tick
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
## Problem
A replica that was a fortified leader at term `T` and has since stepped down re-runs `shouldBcastDeFortify` from `tickElection` once per heartbeat interval, until it learns that an entry from a term `> T` has committed (`InformCommittedTerm`). Each invocation calls `raftLog.term(raftLog.committed)` to fetch the term of the entry at the committed index.
That lookup has no reliable fast path in exactly the situations where the check runs hot:
- The raft **term cache** only covers a suffix of the log and only learns term boundaries from new appends. After a restart it is seeded from the last entry, and on a range that is not committing (while candidates append entries at newer terms), the committed index sits permanently below its coverage. It can never warm for this query.
- The shared **raft entry cache** does cache the loaded entry back, but under mass step-downs the access pattern is a cyclic scan of one distinct entry per obligated replica per second. With O(100k) replicas per store, the working set exceeds the cache by orders of magnitude, so entries are evicted long before the same replica re-checks. Steady-state hit rate is ~0 regardless of runtime, and the scan also evicts entries healthy ranges need.
- On a miss, the fallback reads a **full log entry from pebble** (iterator construction, `SeekGE`, decode, close) to extract 8 bytes of term.
The result: the check performs a storage read per replica per second to re-derive an answer that **cannot change between ticks** (the committed index has not moved — if it had moved past a later-term entry, the obligation would be over).
## Worst-case amplification
In a recent escalation on a large multi-tenant cluster (~100k replicas per node, CPU-constrained), a period of mass leadership churn produced a large standing population of stepped-down ex-leaders on ranges that were leaderless and therefore not committing. For those replicas the exit condition (a later-term commit replicated to them) was unreachable, so the check re-ran indefinitely:
- ~31% of one node's total CPU was inside `shouldBcastDeFortify`, ~93% of that in pebble entry reads; ~11% of the path was spent closing pebble readers.
- The population was replenished by ongoing election churn (every deposed leader adds an obligated replica), making the cost self-sustaining for hours.
- Note the structural trap: the busy loop's termination requires exactly the later-term commit that the CPU it consumes is preventing.
There is an acknowledged TODO about this pattern in `(*Replica).raftTermShMuLocked` referencing #136296.
## Proposed fixes (complementary, in increasing scope)
1. **Memoize the check** (small, immediate): cache the last `(committedIndex, term)` result per replica — or simply skip the lookup entirely while `raftLog.committed` is unchanged since the last check. Invalidation is trivial: the answer can only change when the committed index advances, which is also the only event that can end the obligation. This removes the storage read from the steady-state loop entirely.
2. **Consider skipping de-fortification for terms with no acknowledged fortification**: if the tracker never recorded a single `MsgFortifyLeaderResp`, the broadcast may be unnecessary (a follower that fortified without its response being received self-releases on newer-term contact). Needs a safety analysis; noting it here for completeness.
3. **Cheaper term lookups in general** (#136296): maintain the term index in log storage so a term lookup does not require reading a full entry. Helps other paths with the same shape.
4. **Record the committed entry's term in `HardState`** (suggested by @pav-kv): the leader always knows the committed term (it is the leader's own term when it advances the commit index) and can disseminate it; this makes `term(committed)` an O(1) in-memory read with no log access at all. Suggested as a good starter project for the raft codebase.
Fix 1 is sufficient to defuse the busy-loop cost; 3/4 fix the underlying expense of term lookups below the term cache's coverage.
## Environment
Observed on v26.2.x with leader leases / raft fortification enabled.
Jira issue: CRDB-66545
Contributor guide
Assessment
This issue has not been assessed yet.