Skip to content

db/state, execution: reconcile StateCache and BranchCache at the file-publication boundary - #23047

Draft
yperbasis wants to merge 2 commits into
yperbasis/statecache-followupsfrom
yperbasis/statecache-publication-boundary
Draft

db/state, execution: reconcile StateCache and BranchCache at the file-publication boundary#23047
yperbasis wants to merge 2 commits into
yperbasis/statecache-followupsfrom
yperbasis/statecache-publication-boundary

Conversation

@yperbasis

Copy link
Copy Markdown
Member

Closes #23028.

Stacked on #23033 (draft until it merges, then retargets to main).

Snapshot downloads publish state that never flows through cache applies, so nothing overwrites the entries they invalidate and appliedEnd knows nothing happened. #23033 reconciles the StateCache at one caller — ProcessFrozenBlocks, on its normal exits. Review (codex) found the two gaps that placement leaves, and this PR moves reconciliation to the event itself.

The two gaps

  • Partial failure: publication happens inside RunSnapshots with fallible work after it; an error there returns before the absorb, and ExecModule.Start logs and continues — published files then coexist indefinitely with stale cache entries.
  • The commitment BranchCache was never reconciled (pre-existing on main, not a execution, db: keep StateCache coherent through startup catchup; batch applies #23033 defect): BranchCache.Get guards only against unwind staleness (epoch + floor), and a download advances neither — a branch warmed before startup catchup (engine endpoints are live before Start) can restore the trie from a dead checkpoint and corrupt the first post-snapshot commitment root. Likely backport-relevant (release/3.6 has both the BranchCache and snapshot downloads).

The fix: reconcile where publication happens

recalcVisibleFiles is the chokepoint every publication funnels through (OpenFolder, ReloadFiles, merges, dependency toggles) — the visibility-lowering assert already lives there. It now also reconciles, under dirtyFilesLock and before the new bundle is published, so readers never observe extended files while stale entries live. No caller and no error path can skip it; mid-run downloads are covered for free — which is what closes #23028 rather than just its startup instance.

  • StateCache: BindAggregator's duck contract becomes BindStateCache(*StateCache) — the aggregator holds the cache (legal: execution/cache has no dependency on db/state) and absorbs per-domain values-files ends at every recalculation, plus immediately at bind (files may already be visible when the cache is wired). The frontier needs no transaction, which removes the tx-liveness problem the caller-side placement had. Built-from-applied files stay at or below appliedEnd, so the every-merge path is a strict no-op. execution, db: keep StateCache coherent through startup catchup; batch applies #23033's ProcessFrozenBlocks-local absorb is removed as superseded.
  • BranchCache: gains the applied-watermark it lacked — putWatermark, the exclusive txNum end of the trie's own writes, bumped on every Put. AbsorbFilesExtension(filesEnd) clears everything only when files exceed it: own-built commitment files are a no-op, downloaded state clears. Reconciled unconditionally (the BranchCache is aggregator-owned and commitment correctness is not optional).

Lock order is safe: the hook runs under dirtyFilesLock and takes the caches' internal locks; no path takes them in the reverse order (fills resolve frontiers via atomic bundle loads, applies take only admissionMu).

Testing

Red-first: an aggregator-level publication test (fill a state entry and warm a branch from pre-publication views, generateStateFiles + OpenFolder — both caches must drop their entries and reject pre-publication refills); a bind-after-files-visible test (the admission frontier starts at the published ends); and a BranchCache watermark unit test (own-files no-op, foreign-files clear, no re-clear). Suites: full db/state, execution/commitment under -race (95s), execution/cache + db/state/execctx under -race, execmodule + tester, stagedsync, and the engineapi boot tests (full node boot through OpenFolder with a bound cache — the lock-order claim exercised end to end). make lint clean.

…the file-publication boundary

Snapshot downloads publish state that never flows through cache
applies. The PR-23033 reconciliation ran at one caller
(ProcessFrozenBlocks) on its normal exits only: an error between
publication and the absorb skipped it while Start logs and continues,
and the commitment BranchCache — whose Get guards only against unwind
staleness — was not reconciled at all, so a branch warmed before a
download could restore the trie to the wrong state.

Move reconciliation to the chokepoint every publication funnels
through: recalcVisibleFiles absorbs the bound StateCache (per-domain
values-files ends, no tx involved) and the aggregator-owned
BranchCache, under dirtyFilesLock and before the new bundle is
published, so readers never see extended files while stale entries
live. BranchCache gains the applied-watermark it lacked (putWatermark,
bumped on every Put): own-built files are a no-op, foreign state
clears. BindAggregator's duck becomes BindStateCache — the aggregator
holds the cache and reconciles immediately at bind, covering
wired-after-files-visible ordering; the PFB-local absorb is removed as
superseded. Error paths and mid-run downloads are covered by
construction.

Closes #23028.
The watermark test was written with the boundary-hook commit but never
staged (git add -u skips new files); it sat untracked, breaking
compilation on sibling branches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant