Skip to content

execution/execmodule, node: remove the FCU background flush/commit path - #23051

Open
AskAlexSharov wants to merge 1 commit into
mainfrom
alex/rm_fcu_bg_commit_37
Open

execution/execmodule, node: remove the FCU background flush/commit path#23051
AskAlexSharov wants to merge 1 commit into
mainfrom
alex/rm_fcu_bg_commit_37

Conversation

@AskAlexSharov

Copy link
Copy Markdown
Collaborator

--fcu.background.commit has been default-off since it landed in #18756 (2026-01-23) and was never enabled on any branch — release/3.4, release/3.5, release/3.6 and main all carry FcuBackgroundCommit: false.

It can't be turned on as-is: FCU N returns before its commit lands, so FCU N+1 reads stale state from the DB. That is why its own TestNotificationDispatchBackgroundCommit was skipped, and why the default carries the "needs rawdb API via execctx + Coherent cache revived for rpcdaemon" note. Removing it instead of carrying a dead branch through every FCU change.

What goes

  • --fcu.background.commit flag and ethconfig.Config.FcuBackgroundCommit
  • the bg-commit branch in updateForkChoice: roTx/SD ownership transfer, the goroutine's PublishOverlay(nil) signal, and the extra closeModuleContext on the bad-block path
  • runPostForkchoice's commit half, so it takes only initialCycle (sd, bgRoTx, finishProgressBefore, isSynced were all bg-commit-only)
  • execmoduletester.WithFcuBackgroundCommit

What stays

Flush+commit is now unconditionally inline — identical to the behaviour every node already had. Background prune (--fcu.background.prune, default true) and --sync.parallel-state-flushing are untouched, as is the publishedSD fallback, which the foreground path also uses while a commit is in flight.

Tests

TestInsertBlocksWithBatchedFCU_BadBlockRecovery_{Foreground,Background} collapse into one — both cleanup branches are the same path now. The bg-commit case drops out of TestReorgBackAndForwardIntoCanonicalChain, and the skipped TestNotificationDispatchBackgroundCommit goes with the feature it covered.

go vet ./... compiles clean; ./execution/execmodule/..., ./node/ethconfig/..., ./node/cli/... pass.

The docs/site/versioned_docs/version-v3.4 copy keeps the flag on purpose — 3.4 still has it.

--fcu.background.commit has been default-off since it landed (#18756) and was
never enabled on any release branch: it needs the rawdb API to go through
execctx and the Coherent cache revived for rpcdaemon before it is correct.
Meanwhile FCU N returns before its commit lands, so FCU N+1 reads stale state
from the DB — the reason its own notification-dispatch test was skipped.

Drop the flag, the config field and the bg-commit branch in updateForkChoice.
Flush+commit is now unconditionally inline; background prune is untouched.
runPostForkchoice loses its commit half and its now-unused parameters.

The two bad-block-recovery tests collapse into one (both cleanup branches are
now the same path), and the skipped TestNotificationDispatchBackgroundCommit
goes away with the feature it covered.
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