Skip to content

execution/cache: replace warmup drain-on-unwind with getter-level epoch capture #22116

Description

@mh0lt

Context

PR #21386 added a drainReadAhead (= BlockReadAheader.WaitForWarmup) call before the unwind at all three execmodule entry points (ExecUnwind, set_head, forkchoice). It blocks the unwind until any in-flight block-assembly warmup goroutine finishes, so a fire-and-forget warm Put can't land after the unwind bumped the state-cache epoch and stamp a dead-fork value with the now-live epoch (which the (txNum, epoch) coherence check could then never flag → stale read → wrong root).

That fix is correct but makes the unwind wait on warmup. We'd rather not block the unwind at all.

Why the warmer is coupled to the unwind only through the cache

The read-ahead warmer does not use SharedDomains. Each worker opens its own db.BeginRo and reads through a raw TemporalTx + ReaderV3 (execution/exec/blocks_read_ahead.go), write-through to the process-global *cache.StateCache. Consequences:

  • Its reads are a fixed, isolated MVCC snapshot of committed state — there is no value-source race with the rw unwind.
  • Because it bypasses the SD, it cannot see SD overlay / TemporalMemBatch data — it only ever warms committed backing (fine today: the SD's GetLatest consults its overlay ahead of the cache). Worth keeping in mind if we ever want the warmer to be effective on keys the in-flight block is actively modifying.
  • Its only coupling to an unwind is the shared StateCache's coherence.Gen epoch, which SD.Unwind → stateCache.Unwind bumps. The warm Put self-stamps with coh.Epoch() read at Put time, so a Put that lands after the bump inherits the new epoch — the bug the drain compensates for.

Proposed follow-up (drain-free)

Capture the epoch in the getter instead of draining:

  1. cachePopulatingGetter captures stateCache.Epoch() once at construction (same moment as its RoTx snapshot; the three sub-cache epochs advance in lockstep so one uint32 covers it).
  2. Add PutAtEpoch / PutCodeWithHashAtEpoch variants that stamp with the captured epoch and skip the write if the cache epoch has advanced since capture.
  3. Remove the drainReadAhead calls from the three unwind sites (keep WaitForWarmup for the DB-close path).

Correctness:

  • Stamp-with-captured (not current) closes the wide race: the gap between the getter's epoch check and the actual Put spans a whole file read, so a gate-only check is insufficient — stamping with the captured epoch keeps a straddling Put from laundering a dead-fork value (it lands stamped with the old epoch and IsStale drops the above-floor part).
  • Skip-if-epoch-changed prevents a stale warm Put from clobbering a fresher entry the re-execution wrote post-unwind (a sub-floor warm value could otherwise shadow a key the new fork rewrote above the floor).

Net: the unwind never waits; the in-flight warmer goes inert the moment an unwind bumps the epoch.

Deferred out of #21386 to avoid expanding an already long-running PR.

Metadata

Metadata

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions