Repository navigation
net: fix four ways IBD can stall - #1051
Conversation
- Keep the height and parent of stored path headers through header-cache cleanup, so the download queue can refill. - Do not let a new lookahead header batch jump ahead of stored headers that are waiting for queue space. - Count only empty reconnect rounds with no live peer, and let a replacement connection that already started finish. - Disconnect a peer whose message goes over the rate budget, so a dropped block request is asked again right away. Each fix has a test that fails on master without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Signet run of this branch alone ( |
There was a problem hiding this comment.
Summary
The header-cache retain and the contiguous-enqueue check fix real stalls, and their tests go through hygiene / apply rather than a detached helper. The redial split is also right: an in-flight dial is bounded by connect_timeout, empty rounds no longer abort it, and a fully dark budget still stops new dials and then GiveUpMidCatchup. The rate-limit change does not. It disconnects the follow session on the first over-budget frame without discouraging the address and without releasing asked_blocks, so a dropped body ask is pinned instead of retried. The four changelog fragments also omit the category line release-cut requires.
Issue counts by severity
- bugs: 2
- suggestions: 1
- nits: 0
A follow session drops a peer on its first over-budget message. The session's misbehavior score died with it, so a flooder could reconnect with a fresh score and was never discouraged. The peer hub now keeps a one-hour rate-limit strike per address. A second rate-limit disconnect from that address in the window takes the misbehavior path and refuses the address for a day, as two overflows in one session did before. The strike map uses the discouragement cap and drops expired strikes when it is touched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hub-wide asked set kept every body a session had asked for after the session ended, so other peers skipped those hashes until the set filled and was wiped. The session teardown now forgets its unanswered asks on every exit path, including the new rate-limit disconnect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The redial change lived inline in the IBD loop, and the test only checked the all-peers-dead exit, which master already got right. The two loop decisions are now named functions in exit.rs: an empty round with a live peer resets the dark count, and a fully dark loop stops spawning dials after the limit. The new test walks both and checks that a started dial is still waited out before giving up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The four fragments had no category line, which the release cut rejects, and the release doc asks for one fragment per pull. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Summary
The three earlier findings are fixed on the paths they named. A first application-frame overflow now returns through rate_limit_disconnect, a second strike from that address calls threshold_disconnect (and spawn_inbound_accept refuses the next inbound via inbound_discouraged), and the asked_blocks drain sits after the select so that return releases the session's asks. The changelog is one Fixed fragment, and ibd_cancellable spends the dark budget only through dark_empty_after / may_spawn_redial (exactly four fully dark empty dials, then give up once that dial has finished). Two holes remain: the strike is recorded before the noban/manual/loopback checks, and disconnect_id aborts the session task so the new drain never runs.
Issue counts by severity
- bugs: 2
- suggestions: 1
- nits: 0
A manual, noban, or loopback peer that went over the rate budget still left a strike on its address. The next ordinary peer from that address was then refused for a day on its first overflow. Strikes are now kept only for addresses a misbehavior exit may record, using the same check as the threshold refusal. Also say why the rate-limit exit closes the session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The release ran after the session loop returned, so it was skipped when `disconnect_id` aborted the task (`disconnectnode`, inbound eviction, stale-tip replacement). Those asks stayed hub-wide and other peers skipped the hashes. The follow state now lives in a guard that releases its unanswered asks when dropped, which covers both a normal return and an abort. The post-loop release is deleted. The changelog bullet now says the release covers every way a session ends and that exempt peers leave no strike. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@rearden-grok please re-review the latest commits. |
The test flooded 4001 pings to trip the rate limit before checking that the session's asks were released. Under coverage instrumentation the pings can spread past the one-second window, so the limit never tripped and CI failed. The release now runs from a drop guard on every exit, so the test closes the connection instead. The abort test covers the other exit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Summary
Both bugs from the last review are fixed, and the rate-limit comment now matches the return. refusable_addr is applied before note_rate_strike, so a noban, manual, or loopback overflow still disconnects with Protocol("peer rate limit") and leaves no strike; a normal address strikes once and the next overflow discourages through threshold_disconnect. SessionFollow lives on the peer_session_with task that disconnect_id aborts, and its Drop is the only ask drain, so a normal return and an abort both release requested_blocks (compact and getblocktxn asks included). No new defect in this delta.
Issue counts by severity
- bugs: 0
- suggestions: 0
- nits: 0
Summary
Four bugs can stop IBD from making progress. Each fix is small.
Evidence
Before: each new test fails without its fix:
drained_queue_refills_stored_headers_after_known_header_pruningnew_lookahead_batch_does_not_skip_stored_queue_successorsredial_budget_spends_only_dark_empty_rounds_and_waits_out_a_started_dial(old rule: an empty top-up with a live peer spent the budget, got 4, want 0)rate_limited_block_request_disconnects_without_waiting_for_another_messagesecond_rate_limit_disconnect_from_an_address_discourages_it(second disconnect did not refuse the address)exempt_peer_rate_limit_leaves_no_strike_on_its_address(a manual peer's strike made the next ordinary peer's first overflow a refusal)closed_follow_session_releases_its_block_asks(the ask was still held after the session ended)aborted_follow_session_releases_its_block_asks(the ask was still held afterdisconnect_idaborted the session)After: all pass. fmt, clippy, deny, ast-grep, and os-smoke pass locally. Workspace tests pass except three that also fail on master on this Mac (socket FIN timing,
127.0.0.2bind,end_of_ibd_sh_interrupt).Before: signet IBD from one local peer on
38f419b9: stuck at height 69,190 for 6+ minutes (headers stored=0 queue=1, peer alive). Never recovered.After: a build with this fix synced to 326,785 and went ready. That build also had unrelated perf commits; a run of this branch alone will follow in a comment.
Merge Danger
Door: two-way
Revert the commits to go back. No schema or on-disk change.
Blast Radius: IBD and tip-follow peers
IBD header queueing and peer recovery change. On the tip-follow session, a peer over the rate budget is now disconnected on the first overflow, not the second. The budgets are unchanged. A repeat from the same address within an hour still discourages it for a day. Noban, manual, and loopback peers leave no strike. Strikes are in memory only and capped like the discouraged set.
🤖 Generated with Claude Code