fix(zakura): single-peer sync wedge and block-sync backpressure busy-spin by evan-forbes · Pull Request #435 · valargroup/zebra

evan-forbes · 2026-07-03T16:01:45Z

Motivation

Over the last ~30 commits on ironwood-main, the Unit Tests, Coverage, and
Zakura E2E workflows failed on a large fraction of pushes. #417 quarantined and
retried the worst offenders (the Zakura cluster e2e tests and block-sync fuzzers),
which turned the Unit/Coverage lanes green — but that masked the flakiness rather
than removing it, and the Zakura regtest e2e (pr-gate) lane stayed intermittently
red. This PR fixes the remaining flakiness at its roots and makes the block-sync
fuzzers deterministic so retries are no longer needed.

Two independent root causes were found and fixed:

An intermittent single-peer Zakura sync wedge (drives the pr-gate node4
failure). On a node whose early block progress comes from another source —
inbound gossip or the legacy BlocksByHash path on a dual-stack node — the
Zakura block-sync peer stays "unproven", pinned at the initial one-probe cap,
because that progress never flows through the peer's own body handler. If it is
the node's only peer, the no-progress liveness reaper disconnects it and the node
wedges below the tip with no way to pull the backfill (gossip only pushes new
blocks).
A latent block-sync reactor busy-spin under sustained byte-budget
backpressure — the actual root cause of the block-sync fuzzer flakiness. The
sequencer re-published its progress watch view on every input even when nothing
the reactor/routines schedule against had changed; combined with the per-attempt
floor-funding request, this spun a routine's refill loop with no timer while the
budget was pinned. On a real clock it wasted a core and, under CI load, starved
commit progress enough to trip the deadline-bounded fuzz invariants.

Solution

Fix 1 — single-peer wedge (c9d4b66df, 3eaf8bd6b):

A non-destructive committed-view advance now credits the block-sync peer's
no-progress probe budget when the verified tip advances via any source, so a
progressing node's sole peer keeps probing instead of being reaped.
On regtest, the body-sync stall watchdog falls back to the legacy downloader after
60s (vs the mainnet 10 min), so a stalled node recovers within the regtest e2e
budget. (fix(sync): use verified tip for Zakura stall watchdog #421's watchdog can't fire inside the pr-gate's 180s window; this makes it
effective there while leaving the mainnet timeout unchanged.)

Fix 2 — reactor busy-spin (4f6b8b451):

publish_view now uses send_if_modified: it refreshes the stored view but wakes
watchers only when a schedulable field changed, ignoring the two
observability-only committed_*_per_sec rates (which move on nearly every sample).
This breaks the spin while preserving every real wake (budget release, new work,
frontier advance, apply completion).

Determinism enablers (b54cc343d, dd7066317) and fuzz cleanup (4bff6ad62):

Converged the block_sync module from std::time::Instant to
tokio::time::Instant (Duration stays std). Behavior-identical in production (same
real monotonic clock in a non-paused runtime); it never crosses the module's public
API, so it is not a breaking change. This lets the reactor timers share a test's
virtual clock.
Flipped 19 of 21 block-sync fuzz scenarios to
#[tokio::test(start_paused = true)], so they run to their seeded outcome on an
auto-advancing virtual clock instead of racing real-clock deadlines. The two
worst prior flakes are 20/20 identical after the change.
Removed the fuzz_reorg inverted-xfail landmine (flaked both ways) and the
fuzz_silent_peer_request_cap / fuzz_one_slow_peer_hol assertions already fully
covered by the deterministic block_liveness_* unit tests.

Tests

cargo nextest run -p zebra-network block_sync blocksync_fuzz — 246/246 pass
(220 reactor tests + 21 fuzz + support), 18s.
Determinism: the two worst prior flakes (fuzz_mixed_block_sizes,
fuzz_reliability_discounts_dropping_carrier) and the newly-converted
fuzz_commit_stall each run 15–20× with 0 variance under start_paused.
cargo clippy -p zebra-network --all-targets -- -D warnings clean;
cargo fmt --check clean; cargo check -p zebrad builds.
New regression test block_liveness_external_progress_reopens_unproven_probe_budget
covers the single-peer probe-credit path.

Follow-up Work

Two sustained-backpressure fuzzers (fuzz_multi_peer_tight_budget,
fuzz_peer_that_stops_reading_is_disconnected) stay on real-time multi-thread:
the spin fix removed their wedge but they retain residual virtual-time timing
sensitivity (the wedged-peer case is dominated by the outbound-full poll loop).
Converting them is a follow-up.
The deferred Phase-2 items (backfilling focused reactor tests for the
now-deterministic fuzzers; the supervisor terminate_child LEAK) are not included.

AI Disclosure

AI tools were used: Claude Code — root-cause investigation, the fixes, the
fuzzer-determinism conversion, and this description. The contributor is the
responsible author.

PR Checklist

The PR title follows conventional commits format.
The PR follows the contribution guidelines.
This change was discussed in an issue or with the team beforehand.
The solution is tested.
The documentation and changelogs are up to date.

The main-lane block-sync fuzz suite carried two CI flake sources: - fuzz_reorg was an inverted xfail: it asserted the mock reorg scenario *panics* ("should keep failing until the high-fidelity committer tier lands"). Because the underlying scenario is multi_thread + real-clock, it flaked in both directions and reddened ironwood-main whenever timing let the mock reach the target anyway. Reduce it to a documented #[ignore] no-op; keep its VerifiedReset/slow scaffolding under #[allow(dead_code)] for the future Committer<MockVerifier> tier. - Prune redundant timing-sensitive scenarios whose properties are already covered deterministically in block_sync/tests.rs: delete fuzz_silent_peer_request_cap (covered by the block_liveness_* tests) and drop the flaky protocol_rejects==0 reaper assertion in fuzz_one_slow_peer_hol (covered by block_liveness_progress_before_deadline_keeps_peer_alive).

The mainnet body-sync stall watchdog waits 10 minutes of frozen verified tip before falling back to the legacy ChainSync body downloader. That is far longer than the Zakura regtest e2e pr-gate's ~3-minute catch-up budget, so a node that stalls on Zakura body sync in the pr-gate can never reach the fallback before the harness gives up — the intermittent node4 wedge could not recover in-window even though #421 wired the fallback to verified-tip progress. Add an is_regtest override (60s) mirroring the existing is_regtest restart-delay, so the fallback recovers within the pr-gate. Regtest produces blocks on demand, so a working node still advances every poll and never trips it; falling back early is harmless.

…gress A dual-stack node syncs its early blocks over the legacy BlocksByHash path, which feeds the Zakura commit sequencer. That progress never flows through the Zakura block-sync peer's own body handler, so the peer stays unproven and pinned at initial_block_probe_requests (the one-probe cap). If it is the node's sole peer, the no-progress liveness reaper then disconnects it — right as the legacy feed tapers — and the node wedges below the tip with no way to *pull* the remaining backfill (gossip only pushes new blocks). This is the intermittent regtest-e2e node4 wedge. On a non-destructive committed-view advance, credit the peer's no-progress probe streak when the verified tip advanced via any source, so a progressing node's sole unproven peer keeps probing instead of wedging at the cap. Only the probe streak is cleared; last_block_at (proof) and the liveness deadline are untouched, so a genuinely silent peer is still reaped normally. Mirrors the existing note_view_reset for the advance case. Covered by block_liveness_external_progress_reopens_unproven_probe_budget.

Swap std::time::Instant -> tokio::time::Instant module-wide in the block-sync subsystem (peer routine, registry, admission, BBR, sequencer, service, bench). Duration stays std. Behavior-identical in production: tokio::time::Instant::now() reads the same real monotonic clock as std in a non-paused runtime, so elapsed(), comparisons, and arithmetic are unchanged. Instant never crosses the module's public API (no pub fn takes or returns it), so this is not a breaking change and does not touch any other zakura module. The point is tests: under #[tokio::test(start_paused = true)] the reactor's timers now share the harness's virtual clock, so the in-process block-sync fuzzers can be made deterministic (the reactor previously slept on the tokio clock but measured elapsed with the frozen std clock, causing deadline re-spins and garbage BBR windows under paused time). 220 block_sync tests pass; clippy -D warnings clean.

Flip 18 of the 21 block-sync fuzz scenarios to `#[tokio::test(start_paused = true)]`. With the reactor now on tokio::time::Instant (previous commit), the reactor timers and the in-process harness sleeps share one virtual clock that auto-advances only when every task is parked, so each scenario runs to its seeded outcome in deterministic virtual time instead of racing real-clock deadlines on a loaded CI runner. Verified: the two worst prior flakes (fuzz_mixed_block_sizes, fuzz_reliability_discounts_dropping_carrier) are 20/20 identical; all 18 pass. Three sustained-backpressure scenarios (fuzz_commit_stall, fuzz_multi_peer_tight_budget, fuzz_peer_that_stops_reading_is_disconnected) stay on real-time multi-thread: they drive a reactor<->sequencer<->routine feedback loop that stays perpetually runnable while the byte budget is pinned, so tokio's virtual clock never auto-advances and the scenario livelocks under paused time. Under real time a commit on another worker frees the budget and it converges. This is the same latent busy-spin that makes these three flaky under CI load; a proper fix belongs in the reactor, tracked separately.

The sequencer published its `SequencerView` with an unconditional `send_replace` after every control/body input, marking the `watch` changed even when no field the reactor or per-peer routines schedule against moved. Combined with `reserve_request_budget` sending one `FundFloorReservation` per fill attempt (each handled with a `publish_view`), this created a timer-free reactor<->sequencer<->routine busy-spin whenever the byte budget was pinned: the routine's `sequencer_view.changed()` arm re-fired on an identical view, driving an immediate refill retry that published again. On a real clock this wasted a core and, under CI load, starved commit progress enough to trip the deadline-bounded fuzz invariants (the root cause of the block-sync fuzzer flakiness). Under a `start_paused` test clock it fully wedged: the always-runnable cycle meant the runtime never parked, so virtual time never auto-advanced. Fix: `publish_view` now refreshes the stored view but notifies watchers only when a schedulable field changed, ignoring the two observability-only `committed_*_per_sec` rates (which move on nearly every sample). This breaks the spin while preserving every real wake (a budget release, new work, a frontier advance, or an apply completion all change a schedulable field). With the spin gone, fuzz_commit_stall converts cleanly to deterministic virtual time (15/15 identical). Two sustained-backpressure scenarios (fuzz_multi_peer_tight_budget, fuzz_peer_that_stops_reading_is_disconnected) stay on real-time multi-thread for now: the fix removes their spin but they retain residual virtual-time timing sensitivity (the wedged-peer case is dominated by the outbound-full poll loop). 246/246 block_sync + fuzz tests pass; clippy -D warnings clean.

evan-forbes · 2026-07-03T16:43:15Z

Superseded by the focused split PRs:\n\n- #436: single-peer wedge fix\n- #437: reactor/sequencer no-op wake busy-spin fix\n- #438: fuzzer determinism + cleanup, stacked on #437\n\nClosing this aggregate PR in favor of the split review path.

evan-forbes added 7 commits July 3, 2026 01:06

docs(changelog): note the single-peer Zakura sync wedge fix

05e5ce2

evan-forbes marked this pull request as draft July 3, 2026 16:05

This was referenced Jul 3, 2026

fix(zakura): allow four unproven block probes #436

Closed

fix(zakura): avoid no-op sequencer view wakeups #437

Merged

test(zakura): make block-sync fuzzers deterministic #438

Draft

evan-forbes closed this Jul 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(zakura): single-peer sync wedge and block-sync backpressure busy-spin#435

fix(zakura): single-peer sync wedge and block-sync backpressure busy-spin#435
evan-forbes wants to merge 7 commits into
ironwood-mainfrom
evan/zakura-flaky-fixes

evan-forbes commented Jul 3, 2026

Uh oh!

evan-forbes commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

evan-forbes commented Jul 3, 2026

Motivation

Solution

Tests

Follow-up Work

AI Disclosure

PR Checklist

Uh oh!

evan-forbes commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant