Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ and this project adheres to [Semantic Versioning](https://semver.org).

### Fixed

- Fixed an intermittent Zakura sync wedge on a node with a single block-sync
peer by allowing four initial `GetBlocks` probes before an unproven peer is
capped. A peer still has to deliver an accepted body to become proven, and the
normal no-progress liveness deadline still disconnects peers that do not serve
blocks. In addition, on regtest the body-sync stall watchdog now falls back to
the legacy downloader after 60s (rather than the mainnet 10 minutes) so a
stalled node recovers within the regtest e2e budget.
- Fixed an out-of-memory crash during Zakura block sync when the header chain
runs far ahead of the commit tip. The block-sync applying buffer holds decoded
block bodies ahead of the in-order committer; its look-ahead budget counted
Expand Down
2 changes: 1 addition & 1 deletion zebra-network/src/zakura/block_sync/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub const DEFAULT_BS_FLOOR_RESCUE_TIMEOUT: Duration = Duration::from_secs(2);
/// Request-timeout windows allowed before block-progress liveness disconnects.
const BLOCK_PROGRESS_TIMEOUT_REQUESTS: u32 = 4;
/// Default `GetBlocks` probes sent to a new peer before it proves block-body progress.
pub const DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS: u32 = 1;
pub const DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS: u32 = 4;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed a comment from @p0mvn who was debugging a flakey test

this is in a similar vein to #432, where we're hacking around the problem for now.

why the messages we're sending are not working the way we think they are working is tbd.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #441 where we're gathering more info to debug fully

/// Maximum `GetBlocks` requests sent to one peer without an accepted block body.
pub const DEFAULT_BS_MAX_REQUESTS_WITHOUT_BLOCK_PROGRESS: u32 = 64;
/// Default cooldown before a no-progress peer may be admitted again.
Expand Down
53 changes: 52 additions & 1 deletion zebra-network/src/zakura/block_sync/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,49 @@ fn block_liveness_uses_probe_cap_until_first_accepted_body() {
assert_eq!(window.no_progress_request_cap(), 8);
}

#[test]
fn block_liveness_default_probe_cap_allows_four_unproven_requests() {
let config = ZakuraBlockSyncConfig::default();
let timeout = config.effective_liveness_timeout();
let now = Instant::now();
let mut window = DownloadWindow::new(&config);

for request in 1..DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS {
window.outstanding.push(window_request(request));
window.arm_liveness(now + Duration::from_millis(request.into()), timeout);
assert_eq!(window.requests_without_block_progress, request);
assert!(
window.requests_without_block_progress < window.no_progress_request_cap(),
"the default lets an unproven peer receive another probe"
);
}

window
.outstanding
.push(window_request(DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS));
window.arm_liveness(
now + Duration::from_millis(DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS.into()),
timeout,
);
assert_eq!(
window.requests_without_block_progress,
DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS
);
assert!(
window.requests_without_block_progress >= window.no_progress_request_cap(),
"the fourth unproven request spends the default cap"
);
assert!(
!window.has_block_progress(),
"extra probes must not mark the peer proven"
);
assert_eq!(
window.no_progress_request_cap(),
DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS,
"still unproven: the peer stays on the initial probe cap"
);
}

#[test]
fn block_liveness_resuming_after_idle_gets_fresh_deadline() {
let timeout = ZakuraBlockSyncConfig::default().effective_liveness_timeout();
Expand Down Expand Up @@ -3007,7 +3050,15 @@ async fn block_liveness_disconnects_silent_peer_and_traces_reason() {
bs_trace::BLOCK_GET_BLOCKS_SENT,
&[
("requests_without_block_progress", TraceValue::U64(1)),
("no_progress_request_cap", TraceValue::U64(1)),
("no_progress_request_cap", TraceValue::U64(4)),
("block_progress_proven", TraceValue::U64(0)),
],
);
reader.table("block_sync").assert_row(
bs_trace::BLOCK_GET_BLOCKS_SENT,
&[
("requests_without_block_progress", TraceValue::U64(4)),
("no_progress_request_cap", TraceValue::U64(4)),
("block_progress_proven", TraceValue::U64(0)),
],
);
Expand Down
30 changes: 24 additions & 6 deletions zebrad/src/components/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,18 @@ const GENESIS_TIMEOUT_RETRY: Duration = Duration::from_secs(10);
/// syncer simply finds no new blocks and idles.
const ZAKURA_BODY_SYNC_STALL_TIMEOUT: Duration = Duration::from_secs(10 * 60);

/// Regtest override for [`ZAKURA_BODY_SYNC_STALL_TIMEOUT`].
///
/// The mainnet timeout (10 minutes) is far longer than the Zakura regtest e2e
/// pr-gate's catch-up budget (~3 minutes), so a node that stalls on Zakura body
/// sync in the pr-gate would never reach the fallback before the harness gives
/// up. Regtest produces blocks on demand (not one per ~75s), so a working node
/// still advances its verified tip every poll and never trips this; a genuinely
/// stalled node falls back to legacy `ChainSync` fast enough for the pr-gate to
/// recover within the run. Falling back early is harmless (see
/// [`ZAKURA_BODY_SYNC_STALL_TIMEOUT`]).
const ZAKURA_BODY_SYNC_STALL_TIMEOUT_REGTEST: Duration = Duration::from_secs(60);

/// How often [`ChainSync::bootstrap_genesis_then_pause`] polls the verified tip
/// while watching for Zakura body-sync progress.
const ZAKURA_BODY_SYNC_STALL_POLL: Duration = Duration::from_secs(10);
Expand Down Expand Up @@ -962,10 +974,16 @@ where
);

// Number of consecutive idle polls (no credited progress) that trip the
// fallback. `as_secs` is non-zero for both constants, so this is >= 1.
let max_idle_polls = (ZAKURA_BODY_SYNC_STALL_TIMEOUT.as_secs()
/ ZAKURA_BODY_SYNC_STALL_POLL.as_secs())
.max(1);
// fallback. Regtest uses a much shorter timeout so the e2e pr-gate can
// exercise/recover through the fallback within its short catch-up budget.
// `as_secs` is non-zero for both constants, so this is >= 1.
let stall_timeout = if self.is_regtest {
ZAKURA_BODY_SYNC_STALL_TIMEOUT_REGTEST
} else {
ZAKURA_BODY_SYNC_STALL_TIMEOUT
};
let max_idle_polls =
(stall_timeout.as_secs() / ZAKURA_BODY_SYNC_STALL_POLL.as_secs()).max(1);

let initial_tip = self.latest_chain_tip.best_tip_height();
let mut tracker = ZakuraStallTracker::new(initial_tip);
Expand All @@ -989,7 +1007,7 @@ where
warn!(
verified_tip = ?verified_height,
header_tip = ?header_tip_height,
stall = ?ZAKURA_BODY_SYNC_STALL_TIMEOUT,
stall = ?stall_timeout,
"Zakura body sync is not closing the gap to the network tip; legacy \
fallback disabled (legacy_p2p is off), continuing to wait for Zakura"
);
Expand All @@ -999,7 +1017,7 @@ where
warn!(
verified_tip = ?verified_height,
header_tip = ?header_tip_height,
stall = ?ZAKURA_BODY_SYNC_STALL_TIMEOUT,
stall = ?stall_timeout,
"Zakura body sync is not closing the gap to the network tip; stopping \
Zakura sync drivers and falling back to legacy ChainSync so legacy peers \
can drive body sync"
Expand Down
2 changes: 1 addition & 1 deletion zebrad/tests/common/configs/v5.0.0-rc.3.toml
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ bbr_startup_growth_percent = 200
floor_bypass_slots = 2
floor_peer_avoid_cooldown = "8s"
floor_rescue_timeout = "2s"
initial_block_probe_requests = 1
initial_block_probe_requests = 4
initial_inflight_requests = 64
max_blocks_per_response = 1
max_inflight_block_bytes = 6442450944
Expand Down