fix(zakura): keep Zakura reactors alive when the stall watchdog falls back to legacy sync#481
fix(zakura): keep Zakura reactors alive when the stall watchdog falls back to legacy sync#481p0mvn wants to merge 13 commits into
Conversation
Analyzed three files, diff |
|
Fleet validation: another watchdog-fallback event hit ~30 min after deploy (burst aftermath) — the first one ever survived without human intervention.
No restarts, no operator action, no stranded nodes. Soak continues. |
|
Soak update 2: eu (the last zakura-mode dual-stack node) hit its own watchdog and joined bridge mode — all 5 dual-stack nodes now have One real bug shaken out on the zakura-only node: repeated fork-recovery invalidations (#476 path, firing correctly on min-diff mini-forks) exposed a latent panic in the progress-bar metrics code — |
|
Second gap found and fixed while validating on the zakura-only node (commit 2b6d1eb): if the node exits between a durable header reorg and its body invalidation (which is exactly what the metrics panic caused), later header commits find the rows already replaced, report Meanwhile the bridge design keeps proving itself: after the latest rolling restart, three nodes closed 46-block gaps via pure zakura served by the two bridge-mode nodes, no fallback needed on their side. |
2b6d1eb to
cce28d4
Compare
… back to legacy sync Killing the reactors turned every fallback into a fleet-wide Zakura outage: the fallback node is often the only peer with working block ingest, and with its reactors dead it stops advertising, serving, and forwarding, pinning every Zakura peer's frontier. Legacy ChainSync now resumes as the body-sync driver while Zakura quiesces into a serving/advertising bridge. Also makes inbound block-gossip ingest failures and queue drops visible at info with counters, so tip-ingest freezes are attributable from logs.
…truncated chain tip Invalidating a chain suffix (Zakura fork recovery) can leave last_fork_height above the new tip; recent_fork_length deliberately returns None for that, but the progress-bar metrics caller expected Some and panicked the block write task, crash-looping the node. Metrics display code must never panic.
…ecovery If the node exits between a durable header reorg and its body invalidation, later header commits find the rows already replaced and report no conflict, so the event-based invalidation never fires again and the body chain stays stranded on the abandoned branch forever. Committed ranges overlapping the body tip are now compared against the best body chain, and the first stranded body block is invalidated (one extra state read per commit only when the range overlaps committed bodies).
…rives commits Two sync engines submitting bulk commits concurrently race in the applying queue, so the fallback is now a commit barrier again — without killing the reactors: an apply gate stops new Zakura block applies once fallback engages and the watchdog waits for in-flight applies to finish (bounded; each apply has its own driver timeout) before legacy ChainSync takes the pipeline. Serving, statuses, header commits, and NewBlock accepts continue unaffected.
7fe1a93 to
1aa8ad6
Compare
021af96 to
4d77165
Compare
…h-up Fallback was permanent per process lifetime: bootstrap_genesis_then_pause exited into the legacy sync loop and nothing ever handed the driver role back, so every min-difficulty burst demoted one more node's Zakura sync to a serve-only bridge until restart. The watchdog is now a cycle: on fallback it yields+drains the apply gate, legacy drives until caught up and stable (three consecutive exhausted sync rounds advancing the tip by at most two blocks), then the gate is returned to Zakura and the watchdog resumes. Hand-back happens between legacy rounds, so no legacy bulk work is in flight when Zakura resumes driving.
…dlines When the download floor passed a request (other peers delivered its heights), the GC removed it but only disarmed the liveness deadline for peers whose last delivery was newer than their last request. A healthy peer with an older delivery kept a deadline it could no longer answer, was disconnected for 'no accepted block progress' with zero outstanding, and parked in the 180s no-progress cooldown — at tip this serially exiled peers until the block-sync peer set collapsed (reproduced live 5→0 after a fleet-wide restart, wedging body sync at the next gap). Heights satisfied below the floor now always clear an idle peer's deadline; unresponsive peers are still caught by request timeouts, which deliberately keep the deadline armed.
|
Another live-reproduced fix (commit a433abe), found by the deliberate mass-restart soak: stale block-sync liveness deadlines were serially exiling healthy peers at tip. When the download floor passed a request because other peers delivered its heights, |
The mirror maxed a TipAction::Reset tip against the latest_chain_tip watch, which may not have observed the reset yet; the stale higher tip turned the downstream VerifiedReset into a no-op, pinning the block-sync sequencer one block above the real tip after a fork-recovery invalidation — the missing parent was never requested and every body commit timed out until restart. On Reset the action's tip is authoritative; anti-regression maxing now applies only to Grow.
|
Seventh fix on this branch (600a67e), again live-reproduced within the hour: the chain-tip mirror silently discarded tip resets. On Running tally of live-validated behavior on the full stack tonight: snapshot→tip 12.5k blocks in 152 s hash-equal; 12/12 kill-soak cycles at ~20 s recovery; three hands-off fork recoveries; first hands-off fallback survival; and two at-tip stability bugs (stale liveness deadline cascade, reset-swallowing mirror) found by deliberate mass-restart + tip-idle soaks, both with deterministic regression tests. |
|
Re-promotion validated live on its first natural trigger. testnet-as hit gossip-intake starvation at tip (~52-block freeze — its zakura header store lags at tip by design, so state-driven needed-blocks was empty, and its legacy gossip intake stalled). The full cycle ran hands-off on Also in progress on the same build: the liveness-fix validation — t4 at 8+ minutes of tip-idle with all 5 block-sync peers held and zero liveness disconnects (the old binary's cascade had already started exiling peers by this point). Mass-restart round 2 follows the 20-minute pass. |
|
Mass-restart round 2 (all 6 nodes in the same second, concurrent with a live min-difficulty burst — the harshest combination yet): Validated: no liveness cascade — peer sets were not serially exiled (round 1 collapsed 5→0 in 25 min; round 2 saw only genuine storm-time disconnects). The 20-min tip-idle test beforehand held 5 peers / 0 disconnects / 0 lag. The mirror fix held — no parent-gap zombies. The bridge + re-promotion cycle ran hands-off 2–3 times per dual-stack node through the storm ( Two new findings for the next iteration (documented, not yet fixed):
DoD scorecard on the current stack: snapshot→tip with reorg handling ✅ (12.5k blocks / 152 s / hash-equal, across 3 historical fork points); repeated soak ✅ (12/12 kill cycles + 2 mass restarts, no cascades, no manual interventions); sustained zakura p2p ✅ with the caveat of finding (1) — the layer and driver role now survive and recover from everything thrown at them, but one bounded-stall mechanism remains unidentified. |
|
One last post-storm observation, cleanly isolating a known design gap: t4 is in lockstep at tip but its block-sync peer set is holding at 2 — the storm-parked peers never re-admit because nothing re-escalates an ordered service stream on a live connection, and the #472 keepalives (correctly) removed the accidental refill path (idle-reap → redial → re-escalate). This is the "re-escalation" half of the original §12 Step-1 item, now separated from the winner-agreement half (#458's registration-id work). Next-loop list, in priority order: (1) commit-timeout stage-dump instrumentation to identify the ~45-min verifier-layer stall, (2) suppress no-progress liveness disconnects while local commits aren't landing, (3) service-stream re-escalation on live connections. |
A ~45-minute uniform commit-stall (every Zakura block-sync commit timing out while the state write task, runtime workers, and parent-wait queue were all idle) could not be attributed from logs: the driver's timeout says only that the verifier did not answer. Log the two stages that can silently hold a block — the transaction-verification drain (UTXO awaits on out-of-order parents serialize here, bounded by the 6-minute UTXO_LOOKUP_TIMEOUT) and the state commit await (parent-waiting queue / write-task backlog) — at warn when they exceed 20s, and log timed-out and slow (>20s) transparent UTXO lookups with their outpoint.
…t stalls 'No accepted block progress' cannot be blamed on a peer while the node's own apply pipeline is not landing commits: during a ~45-minute verifier-layer commit stall the liveness deadline exiled healthy peers one by one into the 180s no-progress cooldown (observed live: parks +4 on t2, t4 down to 2 peers). When the sequencer view shows bodies in the applying queue, extend the deadline instead of disconnecting; a genuinely dead peer is caught as soon as the pipeline drains, and its timed-out requests were already requeued by expire_due_timeouts.
|
Backlog item 2 shipped (c3734e5): liveness disconnects are now deferred during local commit stalls. When the sequencer view shows bodies in the applying queue, an expired block-sync liveness deadline extends instead of disconnecting — "no accepted block progress" cannot be blamed on a peer while the node's own commits are not landing (during the ~45-min stall this exiled healthy peers into the 180s cooldown; t4 dropped to 2 peers). A genuinely dead peer is still caught the moment the pipeline drains, and its timed-out requests were already requeued by |
|
Design note closing out the investigation backlog — service-stream re-escalation on live connections (the peer-set refill gap) is documented but deliberately not implemented on this branch. Escalation currently happens exactly once per connection in |
…lures InvalidPeerRange rejections score the peer, but the CommitHeaderRangeError variant behind them was only visible at debug level: a stranded-context wedge (every peer's response for the same range rejected identically for hours, observed live at height 4148005) was indistinguishable from real peer misbehavior at default log levels.
…of scoring peers A fork-recovery re-commit can leave stale rows in the stored difficulty window; every extension range is then rejected with InvalidDifficultyThreshold, each rejection scored and disconnected the honest serving peer, and the wedge survived restarts (on-disk rows). No link failure ever fires in this state, so the existing walk-back trigger never engages. Classify ValidateContextError commit failures as ContextMismatch: never score the peer, and count them as stale-anchor evidence — the quorum (3 failures, 2 distinct peers) starts the same exponential walk-back, and each deepening re-commits a longer range that rewrites the poisoned window.
|
New wedge class found, diagnosed to the exact consensus error, and fixed (9348039): stale difficulty-window rows after a fork-recovery re-commit. t4 wedged at 4148004 for ~1h: every extension range from every peer rejected, each rejection scoring and disconnecting the honest serving peer (59 cycles), surviving restarts — the poison is on disk. The promoted warn log (c9480b5) captured the exact error: Fix: Honest validation note: t4 healed via a naturally-triggered deep walk-back (a real fork during the burst) minutes before the fix binary landed — which live-validates the repair mechanism (deep re-commit rewrites the window) but not the new trigger, which is covered by the unit test and exists precisely for wedges that get no natural rescue. Still open on the diagnosis trail: the write-side hole that left stale rows despite the reorg batch's delete loop ( |
…Mismatch UnknownAnchor cannot be the peer's fault: the anchor is this node's own request bookkeeping, and the error fires when the reactor's in-memory frontier references a hash the header store does not have (observed live at 4148376 after the frontier advanced over diverged store rows). It was classified InvalidPeerRange, so honest peers were scored and disconnected and no recovery trigger ever fired. As ContextMismatch it feeds the stale-anchor quorum; the walk-back re-anchors from the store, which is exactly the self-correction this state needs.
|
Follow-up on the same class (6f8b4ad): a second local-coherence failure surfaced within the hour — |
1 participant
This PR grew during live fleet validation into four related fixes that together make the dual-stack fallback survivable and the fork-recovery pipeline crash-safe. Each was driven by a logged production incident on the zakura testnet fleet (details in the PR comments).
1. Fallback keeps the Zakura reactors alive (the bridge)
Problem: when the stall watchdog fell back to legacy
ChainSync,stop_zakura_synckilled the Zakura header- and block-sync reactors. The fallback node is usually the only peer with working canonical ingest at that moment; killing its Zakura layer stopped its advertising/serving/forwarding, pinned every Zakura peer's frontier at the common stuck height (no higher advertiser → no header requests → no fork-recovery evidence), cascaded the watchdog fleet-wide, and starved zakura-only peers until manual restarts. Observed twice in one night (a min-difficulty burst at ~15 s/block outruns the gossip-only ingest that Zakura mode leaves as the sole canonical intake).Fix: the fallback records the mode switch (
sync.zakura.legacy_fallback.engaged) and resumes legacyChainSyncas the body-sync driver while the reactors stay alive: they follow local commits through the chain-tip mirror and quiesce into a serving/advertising bridge. One bridge node's robust batch-sync ingest then feeds the whole fleet over Zakura.2. Commit barrier without the kill (the apply gate)
Problem: the old hard stop also served as a commit barrier — two engines submitting bulk commits concurrently race in the applying queue (a previously-observed failure class). Removing the kill alone reopened that race in the handoff window.
Fix:
ZakuraApplyGate— on fallback, no new Zakura bulk applies start and the watchdog drains in-flight applies (bounded backstop; each apply has its own 30 s driver timeout) before legacy takes the pipeline. Permits are threaded through the apply futures so the barrier is exact. Serving, statuses, header commits, andNewBlockaccepts (the same class as gossip commits, which already coexist withChainSync) continue.3. Metrics code must not panic the write task
Problem: repeated fork-recovery invalidations exposed a latent panic:
update_metrics_bars.expectsrecent_fork_length()after checkingrecent_fork_height(), but invalidating a chain suffix can leavelast_fork_heightabove the truncated tip — whererecent_fork_length()deliberately returnsNone. The panic killed the block write task →chain tip sender was dropped→ process exit → crash loop on a zakura-only node. Also reachable via theinvalidateblockRPC.Fix: the metrics caller handles
None.4. Reconcile a body suffix stranded across a crash (
ReadRequest::ZakuraHeaderHash)Problem: if the node exits between a durable header reorg and its body invalidation (exactly what the panic above caused), later header commits find the rows already replaced, report
reorged_at: None, and the event-based invalidation never fires again — headers honest, bodies permanently stranded, walk-back cycling uselessly.Fix: on the recurring
BodyGapssignal, the driver compares the best body chain against the Zakura header store (newReadRequest::ZakuraHeaderHash, which reads the header store directly — the serving reads prefer the best body chain and would mask the divergence), walks down to the fork point, and invalidates the first stranded body block (sync.header.fork_recovery.body_suffix_reconciled). Two reads when the chains agree; zero writes.Plus ingest observability: gossip download/verify failures at info with counters (
gossip.ingest.{failed,duplicate}.count), queue/lookahead drops promoted to info — the previous silence made tip-ingest freezes unattributable from fleet logs.Tests
fallback_drains_the_apply_gate_without_cancelling_zakuraet al.).stranded_body_suffix_is_reconciled_by_invalidation: mock-state agree/stranded cases, including the fork-point walk-down.zebra-network(897),zebradzakura (53) +components::sync(50) + inbound (22),zebra-state(301); fmt + clippy clean.body == headerthroughout, zero reactor exits); nodes closing 46-block gaps over pure Zakura served by bridge nodes; the crash-loop reproduced and eliminated; the stranded suffix reconciled live (node unstranded ~470 blocks in seconds); and with the full stack: snapshot→tip onboard of 12,529 blocks in 152 s (~7,400 blocks/min) ending hash-equal with the fleet.Follow-up Work
9b86d526, needs block-sync coverage).AI Disclosure
PR Checklist
type(scope): description