erigontech / erigontech/erigon
execution, db: simplify unwind read-ahead and overlay transaction plumbing
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Goal
After #23005, evaluate two independent simplifications. Neither is required for its correctness, and each should be implemented only if tests preserve the final unwind invariants.
## Remove explicit read-ahead suspension
#23005 gives read-ahead fills the same state-version and `readViewEpoch` admission as other readers, and repeats unwind invalidation when the database commit becomes durable. This may make the separate `warmupGate`, `SuspendWarmup`, and `ensureReadAheadSuspended` plumbing unnecessary.
Before removing it, tests must cover every boundary:
- A warmup view created before unwind staging loses fill authority when the epoch advances.
- A warmup started during a staged unwind cannot leave dead-fork entries after `PublishUnwind`.
- A view opened after the database commit but before cache publication cannot fill against the older cache version.
- An abandoned validation may leave only values that remain valid for the unchanged durable state.
- Shutdown still waits for active warmup goroutines without waiting for an unrelated FCU suspension.
If those properties hold under the race detector, remove the suspension callback and semaphore ownership transfer across `execmodule`, `ForkValidator`, and `BlockReadAheader`. Measure FCU latency and cache warmup effectiveness before and after. If a counterexample remains, document it in the cache admission contract and keep the gate.
#23003 is the narrower alternative: it retains suspension but moves it closer to actual unwinds.
## Store the `MemoryMutation` backing transaction once
`MemoryMutation.readTx` and `MemoryMutation.db` represent the same backing transaction. `db` is only the optional `kv.TemporalTx` view of `readTx`, so constructors, `UpdateTxn`, `DetachDB`, and read-view construction must keep two fields synchronized.
Store one backing `kv.Tx` and derive the temporal capability through a small typed helper, or use another representation with one source of truth. Preserve these contracts:
- Plain overlay read views may use a non-temporal `kv.Tx`.
- Temporal methods delegate only when the backing transaction implements `kv.TemporalTx`.
- Detached overlays fail or use memory-only behavior consistently instead of dereferencing a missing backing iterator.
- Transaction replacement, detachment, and access follow one clear locking rule.
#23254 separately tracks synchronizing `UnderlyingTx`; this simplification should preserve or complete that fix rather than weaken its lifetime rules.
## Completion criteria
- Keep the two changes independently reviewable and benchmark the read-ahead change separately.
- Add focused regression tests before changing behavior.
- Run the affected execution and `membatchwithdb` packages under the race detector.
Contributor guide
Assessment
This issue has not been assessed yet.