[r3.6] execution/commitment: fix wave-BFS livelock when a step budget fills exactly - #23153
Merged
lystopad merged 1 commit intoAug 11, 2026
Merged
Conversation
…exactly (#23066) ## Problem `ContractTrunkPreloadParallel.Run` can spin forever in its wave loop, wedging the node. The loop only breaks on `budgetHit`, which `pin()` raises on a *strict* overflow. A wave that pins nothing never raises it, and the frontier is reassigned at the same depth — identical state on re-entry. Two ways a wave pins nothing: - **Whole miss set deferred.** `fileBudget` on re-entry is `stepCap - (usedBytes + dbHitsBytes)`, bit-identical to the value that caused the deferral, because the db-hits consume exactly the bytes the deferral already accounted for. Infinite. - **Capped fetch resolves to nothing.** Every fetched key is absent from the file layer — the normal shape at the BFS fringe, where a set `afterMap` bit names a leaf with no branch record. Re-enters once per chunk: bounded, but quadratic. ## Impact Hit in production on a Gnosis validator (`release/3.6`, 4402a1d): ``` sortAndPartitionFrontier preload_parallel.go:121 Run preload_parallel.go:216 runExtensionLocked adaptive_pin.go:354 OnBlockComplete adaptive_pin.go:206 SharedDomains.Commit domain_shared.go:1030 updateForkChoice forkchoice.go:742 ``` The stuck goroutine holds the execution semaphore inside an FCU, so `NewPayload` blocks while holding the fork-choice write mutex. The CL slot ticker stops and ~420 `getAttesterDuties` handlers pile up on the read lock. The node stopped attesting and producing for 17h while burning a core; `eth_syncing` still reported `false`. Two dumps minutes apart showed the same goroutine id, receiver and `dbBranches` pointers — livelock, not a slow walk. 68,732 extension runs over 5.9d uptime before it hit. ## Fix End the step when a wave defers part of its miss set and pinned nothing. Each iteration now either breaks, strictly shrinks the frontier, or increments `nextDepth`. Deferred work resumes on the next `Run`, so there is no throughput cost. Also: - Floor the file fetch at `minEntryBytes` — below one entry's cost the batch is pure waste. - Add the missing `cache == nil` check to `PreloadContractTrunkParallel`: `Run` returns the error, then the wrapper's logger block dereferences `cache.PinnedCount()`, so callers got a panic instead. - Fold two disagreeing `queueEmpty` expressions into one method. ## Testing Differential run of the fixed `Run` against the pre-guard one over 40k configs / 2.3M steps: identical pins, `queueEmpty` and `usedBytes` at every step. The pre-guard code livelocked in 18,306 of those configs. Thanks @awskii. - `ExactBudgetFillTerminates` — the production state; hangs before the fix. - `NoPinWaveEndsStep` — both no-pin paths as two budgets one byte apart, asserting the exact resolver call count. Each case fails against exactly one of the two production changes reverted, so neither is left unpinned by the other. - `DeferWithDbHitsInSameWave` — deferral boundary with db-hits pinned in the same wave. - `StepBudgetSweepTerminates` — budgets straddling one entry's cost. Hang guards abort the binary rather than `t.Fatal`, since `Run` has no cancellation and a spinning goroutine would outlive the failure. `execution/commitment` green under `-race -count=2`; `make lint` clean. ## Scope `preload_parallel.go` is the only site with this shape: the serial `ContractTrunkPreload.Run` pops its queue head unconditionally, `OnBlockComplete` ranges a bounded set, and `execStatusList.drainDeferred` already has a progress net. `release/3.6` needs a backport; `release/3.5` does not have the feature. ## Notes (not fixed here) - `adaptive_pin.go`: `c.misses` entries are never removed, so one `sync.Map` entry accumulates per contract hash ever touched and `OnBlockComplete` ranges the whole set every block. Found by @awskii. - `commitment_trunk_preload_{bytes,duration_seconds}_total` are declared but never written — no metric signal for preload work while this was wedged. Fixed in #23067. - When `PerContractMaxBudgetBytes - usedBytes` drops below one entry's cost, `runExtensionLocked` runs every block and pins nothing. Bounded per-block work, not a hang. --------- Co-authored-by: awskii <artem.tsskiy@gmail.com>
AskAlexSharov
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #23066 to release/3.6.
Applies cleanly; both files are blob-identical to the merged commit (3be2c59), so no r3.6-specific adaptations were needed.
release/3.6 is the branch this was hit on in production (4402a1d).