rpc/jsonrpc: hold an undecided pre-merge probe for a short TTL - #24218
Merged
Merged
Conversation
_preMergeData is stored only when the probe decides, so a datadir whose block data cannot be read was never remembered and the walk ran again on every gated RPC: MinimumBlockAvailable plus about log2(mergeHeight) CanonicalBodyForStorage reads, each one a gRPC round trip on a remote rpcdaemon. An undecided answer is now held for a TTL of its own, one second, separate from the 30s verdict TTL: the verdict expires because the segment set widens, the non-verdict because the data it waits for can arrive at any time. A fresh verdict is still checked first, and only a probe that neither decided nor failed is recorded, so a failed read is still not inherited by another caller. The question stays unsettled: an undecided probe is held, never stored as an observation, so a datadir whose blocks have not arrived is still not read as an archive one.
Collaborator
|
Lean already. Ship. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
An undecided refresh can renew a previous verdict for 30 seconds, bypassing the intended one-second TTL.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds short-lived caching for undecided pre-merge probes to avoid repeated remote block reads.
Changes:
- Adds a separate one-second TTL for undecided probe results.
- Adds tests for reuse and expiry of undecided results.
| File | Description |
|---|---|
rpc/jsonrpc/eth_api.go |
Implements unsettled probe caching. |
rpc/jsonrpc/pre_merge_probe_test.go |
Tests unsettled-result TTL behavior. |
rpc/jsonrpc/check_prune_gates_test.go |
Disables the new TTL for an existing gate test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
AskAlexSharov
approved these changes
Sep 22, 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
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.
Review follow-up from #23322:
#23322 (comment)
_preMergeDatais stored only when the probe decides, so a datadir whose blockdata cannot be read was never remembered and the walk ran again on every gated
RPC:
MinimumBlockAvailableplus about log2(mergeHeight)CanonicalBodyForStoragereads, each one a gRPC round trip on a remoterpcdaemon. #23939 closed the
earlyTxnSpentbranch of the same note; thiscloses the remaining one.
An undecided answer is now held for a TTL of its own, one second, separate from
the 30s verdict TTL. A fresh verdict is still checked first, and only a probe
that neither decided nor failed is recorded, so a failed read is not inherited
by another caller. The answer is held, never stored as an observation, so a
datadir whose blocks have not arrived is still not read as an archive one.
Cost: up to one second before arriving blocks are noticed, inside the window
where the gate rejected them anyway.
Tests:
TestPreMergeProbeHoldsAnUnansweredWalkForItsTTLpins the reads (20across 5 requests before, 4 after);
TestPreMergeProbeWalksAgainOnceTheUnansweredTTLPassespins the otherdirection.
TestBlocksGateDoesNotSettleExpiryBeforeBlocksArrivesets the newTTL to zero, so it keeps observing one walk per call.