erigontech / erigontech/erigon
db/kv/kvcache, execution/execmodule: Coherent warmth recovery — carry-over prerequisites (beyond #22276) and unannounced-version View waits
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Context
#22532 (split out of #21293) fixed announced-vs-committed `PlainStateVersion` parity and reworked `kvcache.Coherent` so canonical roots start fresh from their own batch. That is the correct call while the state-change producers are lossy, but cross-block cache warmth is gone: every version starts cold and only intra-version reads hit. #22276 tracks making the producers complete and ends with "re-enable cross-block cloning in `advanceRoot`" — this issue tracks the cache- and dispatch-side work that re-enabling *also* requires (reverting the `advanceRoot` hunk alone is neither sufficient nor fully effective), plus an independent wait-latency gap for versions that commit without an announce.
Producer companion: #22276. Groundwork PR: #22532. Default flip: #22269.
## Work items
### 1. Clone from the previous canonical root, not `roots[id-1]`
The pre-#22532 carry-over keyed on numeric adjacency: `c.roots[stateVersionID-1]`. Announced versions do not step by 1: the serial exec path bumps `PlainStateVersion` once per block (`computeAndCheckCommitmentV3`, `execution/stagedsync/exec3.go`) in addition to the once-per-flush bump (`TemporalMemBatch.flushLocked`), so consecutive announces differ by 2 for single-block serial FCUs (K+1 for a K-block FCU). Under serial exec the old clone condition never held — carry-over was silently dead there and only fired under parallel exec by arithmetic accident.
When re-enabling, clone from `c.roots[c.latestStateVersionID]` gated on `isCanonical`. The precondition is batch completeness (#22276), not version contiguity.
### 2. `Action_REMOVE` / re-creation must invalidate derived entries
With carry-over, a REMOVE nils only the account entry; the address's code entry (keyed by address since #22532) and cached storage slots (`addr+loc`) inherited from prior roots survive as stale non-nil values. The batch cannot enumerate deleted slots (a pre-Cancun selfdestruct can have unbounded storage), so this must be handled consumer-side in `OnNewBlock`: on REMOVE, nil the code entry and prefix-delete `addr…` from the root's storage btree — bounded by what is actually cached. Incarnation re-creation (gap 2 in #22276: `DomainDel(Code)` + `DomainDelPrefix(Storage)` announced only as `ChangeAccount`) needs the same invalidation; coordinate the producer-side signal (dedicated marker vs REMOVE+UPSERT pair) with that fix.
### 3. Restore the clone-keeps-eviction-lists fast path
`advanceRoot` currently rebuilds `stateEvict`/`codeEvict` by walking the root's btrees. Near-empty fresh roots make that cheap today; a carried multi-GB root would make it O(cache) per block. The old clone branch left the lists untouched (the COW `Copy()` shares `Element` pointers) — restore that path together with the clone.
### 4. Stop `View` waits on versions that will never be announced (independent — can land before #22276)
Commits that bump `PlainStateVersion` without a dispatch leave readers on unannounced versions; each `View` waits `NewBlockWait` (5 ms default), up to `MAX_WAITS` (100) per window before the circuit breaker trips, and the breaker resets on every `OnNewBlock`. Two producer paths in `updateForkChoice` (`execution/execmodule/forkchoice.go`):
- the `isDomainAheadOfBlocks` recovery branch — commits, returns `TooFarAway`, never dispatches;
- catch-up `CommitCycle` commits (the callback passed to `PipelineExecutor.RunLoop`) — every cycle commits and bumps; only the final version is announced.
Two complementary fixes:
- **Announce every committed version.** A header-only `StateChangeBatch` (`SetStateID` + `StartChange(currentHeader, nil, false)`, no entries — the shape `Dispatcher.Dispatch` already produces when the accumulator is empty) after each such commit. `OnNewBlock` handles entry-less batches (creates the root, closes `ready`). This also removes unfed holes in the carry-over chain once cloning returns. Verify other StateChanges consumers (txpool) tolerate header-only batches at catch-up cadence.
- **Consumer fast-path.** In `View`, skip the wait when `id < c.latestStateVersionID`: announces are monotonic, so a root older than latest can never become ready — waiting is pure loss for laggard snapshots.
## Sequencing / acceptance
- Item 4 is independent and can precede #22276.
- Items 1–3 land together with #22276's producer fixes, gated on that issue's differential completeness test (shadow-map applied batches vs committed state per version, with reorgs, selfdestructs, and incarnation reuse in the mix).
- Each item carries its own unit test in `db/kv/kvcache`; `TestCanonicalRootsStartFresh` (which pins the current no-carry-over behavior) gets replaced by carry-over tests asserting REMOVE/re-creation invalidation.
- Measure before prioritizing items 1–3: `cache_total{result="hit|miss"}` on a #22269 shadow run shows whether per-version warmth already suffices for real traffic. `Coherent.ValidateCurrentRoot` is the ready-made differential checker to run in a devnet job while carry-over soaks — once #22277 fixes it.
Nice-to-have while in the area: `getFromCache` serializes all reads on one mutex (deliberately, RLock caused `runtime.usleep` degradation); if the flip makes hit traffic hot, shard the lock by key along the lines of commitment's sharded `BranchCache`.
Contributor guide
Assessment
This issue has not been assessed yet.