[domiwei]

Problem

On Gloas the ForkChoice stage runs again for the same head root when the head's payload status changes from EMPTY to FULL (GetExecutionHead changes and sleep_for_slot.go wakes the stage), and again about once per second while the forkchoice update returns busy. Each run calls postForkchoiceOperations, which dumps the head state with DumpBeaconStateOnDisk. The envelope does not change the beacon state, so every extra run encodes and rewrites the same state for the same root.

A head-state dump takes about 405 ms p50 on a mainnet node with the current dump code (no fsync since #25251), and it holds stateDumpLock, which disk state reads (replays) also need. By code reasoning this can add up to 8 redundant dumps per epoch on Gloas (one per dump slot, more while the forkchoice update is busy). There is no Gloas mainnet data for this yet.

Fix

forkGraphDisk remembers the block root and slot of its last successful state dump, guarded by stateDumpLock. DumpBeaconStateOnDisk returns early when both match. The slot is part of the key because a state advanced through empty slots keeps its block root but has different content. The marker is cleared before each write and set only after the write finished and the file closed without error, so a failed or panicked write is retried (stage panics are recovered, so the next run would otherwise skip a truncated file).

Tests

New tests in fork_graph_test.go count state-file writes through the existing failing-fs wrapper:

• the same root and slot dumped twice writes once (red on main: 2 writes);
• a close error is retried (TestDumpBeaconStateReturnsCloseError);
• a panic during the write is retried (TestDumpBeaconStateRetriesAfterPanic);
• a different root is still written;
• the same root at a different slot is still written, and the file holds the later state;
• a failed later-slot write does not block a new dump of the earlier slot.

Each test fails when the production branch it targets is removed.

Out of scope

• stateDumpLock contention between state dumps and replays in general, and making the state dump atomic (temp file + rename).
• Avoiding the repeated ForkChoice stage run itself.

Related to #25120 and #25136.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the change to suppress redundant Caplin head-state dumps.

No open issues 🎉

_{Review effort: Balanced}

[yperbasis]

Approving at 6b00deb.

Two optional nits:

• The struct comment "reusable disk I/O buffers, each guarded by the mutex above it" now also heads lastDumpedRoot and lastDumpedSlot, which are not buffers.
• Clearing only lastDumpedRoot before a write leaves a stale lastDumpedSlot. A zero block root cannot occur, so this is harmless, but clearing both fields would make the "nothing dumped" state explicit.

Both commits cherry-pick cleanly onto release/3.8 and the new tests pass there. The related performance fixes were all backported, so this one probably should be too.

[domiwei]

@yperbasis thanks, both nits are addressed:

• The struct comment: each mutex now has its own comment. stateDumpLock guards the reusable state buffer and the key of the last state dump, and envelopeMu guards the reusable envelope buffer (4a52476, 31582b1). Comment-only change.
• Both lastDumpedRoot and lastDumpedSlot are now cleared before the write (a0a7ef6), so a failed or panicked write leaves the same zero state as a fresh forkGraphDisk. As you said, a block root is never zero, so behaviour does not change; the existing retry tests still pass.

Backport: agreed, I will open a backport PR to release/3.8 after this one merges. With the new commits, all five commits still cherry-pick cleanly onto release/3.8, and the fork_graph tests pass there.

[Copilot]

🟢 Approval recommended

There are no active review issues for the beacon-state dump deduplication change.

No open issues 🎉

_{Review effort: Balanced}