Skip to content

Commit f3da758

Browse files
julianpryderamfox
andauthored
fix(iroh): add jitter to dns retry calls (#3447)
## Description Adds a random jitter to DNS retries called with the `stagger_call` function. ## Breaking Changes N/A ## Notes & open questions Further research can determine if additional timeout jitters are needed when calling the other DnsResolver public functions. Nominal 20% of default delay time as maximum deviation from original timeout delay. Real world and lab testing can dermine if this is sufficient. Partially addresses #3017. ## Change checklist <!-- Remove any that are not relevant. --> - [x] Self-review. - [x] Tests if relevant. --------- Co-authored-by: ramfox <kasey@n0.computer>
1 parent fa1e946 commit f3da758

File tree

1 file changed

+43
-1
lines changed

1 file changed

+43
-1
lines changed

iroh-relay/src/dns.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ pub const N0_DNS_NODE_ORIGIN_PROD: &str = "dns.iroh.link";
2828
/// The n0 testing DNS node origin, for testing.
2929
pub const N0_DNS_NODE_ORIGIN_STAGING: &str = "staging-dns.iroh.link";
3030

31+
/// Percent of total delay to jitter. 20 means +/- 20% of delay.
32+
const MAX_JITTER_PERCENT: u64 = 20;
33+
3134
/// Potential errors related to dns.
3235
#[common_fields({
3336
backtrace: Option<Backtrace>,
@@ -503,7 +506,7 @@ async fn stagger_call<
503506
// NOTE: we add the 0 delay here to have a uniform set of futures. This is more performant than
504507
// using alternatives that allow futures of different types.
505508
for delay in std::iter::once(&0u64).chain(delays_ms) {
506-
let delay = Duration::from_millis(*delay);
509+
let delay = add_jitter(delay);
507510
let fut = f();
508511
let staggered_fut = async move {
509512
time::sleep(delay).await;
@@ -523,6 +526,19 @@ async fn stagger_call<
523526
Err(StaggeredError::new(errors))
524527
}
525528

529+
fn add_jitter(delay: &u64) -> Duration {
530+
// If delay is 0, return 0 immediately.
531+
if *delay == 0 {
532+
return Duration::ZERO;
533+
}
534+
535+
// Calculate jitter as a random value in the range of +/- MAX_JITTER_PERCENT of the delay.
536+
let max_jitter = delay.saturating_mul(MAX_JITTER_PERCENT * 2) / 100;
537+
let jitter = rand::random::<u64>() % max_jitter;
538+
539+
Duration::from_millis(delay.saturating_sub(max_jitter / 2).saturating_add(jitter))
540+
}
541+
526542
#[cfg(test)]
527543
pub(crate) mod tests {
528544
use std::sync::atomic::AtomicUsize;
@@ -548,4 +564,30 @@ pub(crate) mod tests {
548564
let result = stagger_call(f, &delays).await.unwrap();
549565
assert_eq!(result, 5)
550566
}
567+
568+
#[test]
569+
#[traced_test]
570+
fn jitter_test_zero() {
571+
let jittered_delay = add_jitter(&0);
572+
assert_eq!(jittered_delay, Duration::from_secs(0));
573+
}
574+
575+
//Sanity checks that I did the math right
576+
#[test]
577+
#[traced_test]
578+
fn jitter_test_nonzero_lower_bound() {
579+
let delay: u64 = 300;
580+
for _ in 0..100 {
581+
assert!(add_jitter(&delay) >= Duration::from_millis(delay * 8 / 10));
582+
}
583+
}
584+
585+
#[test]
586+
#[traced_test]
587+
fn jitter_test_nonzero_upper_bound() {
588+
let delay: u64 = 300;
589+
for _ in 0..100 {
590+
assert!(add_jitter(&delay) < Duration::from_millis(delay * 12 / 10));
591+
}
592+
}
551593
}

0 commit comments

Comments
 (0)