Skip to content

execution, db: simplify unwind read-ahead and overlay transaction plumbing #23257

Description

@yperbasis

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.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions