node: stop transport errors from poisoning blocks_by_range peers (+ TTL) - #1050
Merged
Merged
Conversation
…d TTL
A recurring catch-up wedge (observed on devnet zeam_4, 8995 slots behind
with every range-capable peer marked "cannot serve blocks_by_range")
traced to two coupled defects in the blocks_by_range availability logic.
1. Misclassification. `isBlocksByRangeUnavailable` short-circuited on a
bare RPC error code 1 (INVALID_REQUEST). But a mid-stream QUIC
teardown / IO error / timeout while a range request is in flight also
surfaces as code 1, with a transport message ("Disconnected",
"IoError", "peer disconnected"). Those transient failures were
classified as a permanent "peer lacks blocks_by_range" and poisoned
the peer. A node that fell behind issued large range requests, the big
transfers hit teardowns, and each one marked a still-connected,
range-capable peer unavailable until none remained — then the gap grew
past MAX_BLOCKS_BY_ROOT_CATCHUP_GAP and catch-up wedged. A peer that
stayed synced never triggered large requests and never entered the
spiral, which is why only some nodes wedged.
Fix: treat transport messages as transient (never unavailable); only an
explicit "unsupported protocol" reply, or a bare code-1 with a
non-transport message, marks a peer as lacking the protocol.
2. No reset/TTL. The `blocks_by_range_unavailable` mark was sticky and
only cleared on reconnect, so a wrongly-marked peer that stayed
connected was excluded from range sync for the life of the connection —
exactly the failure the marking path's own comment warned about but
never guarded. Replace the bool with a timestamp and expire the mark
after BLOCKS_BY_RANGE_UNAVAILABLE_TTL_S (300s), so any misclassification
self-heals in minutes instead of persisting.
Adds unit tests: transport messages are not flagged unavailable, and the
mark expires after its TTL without a reconnect.
Contributor
|
Approved from my side. I did an adversarial pass over #1050 and found no blocking issues. What I checked:
Validation I ran locally:
I also started |
noopur23
approved these changes
Jul 18, 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.
Problem
Recurring catch-up wedge on devnet (
zeam_4: 8995 slots behind, every peer loggingcannot serve blocks_by_range and gap=… exceeds the blocks_by_root walk limit (64)).zeam_6never wedged.Root cause
The devnet
blocks-by-range request … failedlines show the poisoning trigger:These are transport-level failures (mid-stream QUIC teardown / IO error) that surface as RPC error code 1.
isBlocksByRangeUnavailableshort-circuited on barecode == RPC_ERR_INVALID_REQUEST→ misclassified them as "peer doesn't support blocks_by_range" → set the stickyblocks_by_range_unavailableflag.Self-reinforcing spiral: a node that fell behind issued large
blocks_by_rangerequests → big transfers hit stream teardowns → each poisoned a range-capable peer → eventually no range peers left → gap grew pastMAX_BLOCKS_BY_ROOT_CATCHUP_GAP(64) → wedge. Nodes that stayed synced never issued large requests, so they never entered the spiral — that's thezeam_4vszeam_6difference.Compounding it: the flag was only reset on reconnect (
connect()rebuildsPeerInfo). A peer that stayed connected after being wrongly marked was excluded from range sync for the life of the connection — the exact failuremarkBlocksByRangeUnavailable's own comment warned about ("add a reset/TTL when you do") but never guarded.Fix
isBlocksByRangeUnavailable): transport messages (disconnect,ioerror,timeout,reset,closed,cancel,eof, …) are transient and never mark a peer unavailable. Only an explicit "unsupported protocol" reply, or a bare code-1 with a non-transport message, counts.BLOCKS_BY_RANGE_UNAVAILABLE_TTL_S = 300): the mark is now a timestamp and expires after 5 min even without a reconnect, so any misclassification self-heals in minutes instead of persisting for the connection lifetime.Tests
isBlocksByRangeUnavailable does NOT flag transport failures(IoError / Disconnected / peer disconnected / reset / timeout / closed).zig build testgreen.Deploy note
Deploying requires a rebuild + container restart; a restarted
zeam_4starts with fresh (unpoisoned) peer state, and with this fix transport blips no longer re-poison, so it can range-sync out of the 8995-slot gap.