erigontech / erigontech/erigon

db/kv/kvcache, execution/execmodule: Coherent warmth recovery — carry-over prerequisites (beyond #22276) and unannounced-version View waits

Open
#22,290 1 comment 0 reactions 1 assignee Claimed by @yperbasis View on GitHub
ErigonDB performance
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.