From e4cb6ecc151b935be04f169e65e79281c8e37f8e Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Wed, 23 Feb 2022 14:56:47 -0800 Subject: [PATCH] Add ExponentialBackoff::new_unchecked (#1517) Now that we can use const functions, we can update `ExponentialBackoff` to expose a constructor instead of having public members. Signed-off-by: Oliver Gould --- linkerd/app/inbound/src/test_util.rs | 2 +- linkerd/app/outbound/src/test_util.rs | 2 +- linkerd/app/src/env.rs | 16 ++---- linkerd/exp-backoff/src/lib.rs | 72 ++++++++++++++------------- 4 files changed, 45 insertions(+), 47 deletions(-) diff --git a/linkerd/app/inbound/src/test_util.rs b/linkerd/app/inbound/src/test_util.rs index 35d1ae9757..d228fe4ba5 100644 --- a/linkerd/app/inbound/src/test_util.rs +++ b/linkerd/app/inbound/src/test_util.rs @@ -32,7 +32,7 @@ pub fn default_config() -> Config { connect: config::ConnectConfig { keepalive: Keepalive(None), timeout: Duration::from_secs(1), - backoff: exp_backoff::ExponentialBackoff::new( + backoff: exp_backoff::ExponentialBackoff::try_new( Duration::from_millis(100), Duration::from_millis(500), 0.1, diff --git a/linkerd/app/outbound/src/test_util.rs b/linkerd/app/outbound/src/test_util.rs index 4afd5c5a88..1bf8fcb003 100644 --- a/linkerd/app/outbound/src/test_util.rs +++ b/linkerd/app/outbound/src/test_util.rs @@ -26,7 +26,7 @@ pub(crate) fn default_config() -> Config { connect: config::ConnectConfig { keepalive: Keepalive(None), timeout: Duration::from_secs(1), - backoff: exp_backoff::ExponentialBackoff::new( + backoff: exp_backoff::ExponentialBackoff::try_new( Duration::from_millis(100), Duration::from_millis(500), 0.1, diff --git a/linkerd/app/src/env.rs b/linkerd/app/src/env.rs index 2192542598..52bc3125f5 100644 --- a/linkerd/app/src/env.rs +++ b/linkerd/app/src/env.rs @@ -227,19 +227,13 @@ const DEFAULT_METRICS_RETAIN_IDLE: Duration = Duration::from_secs(10 * 60); const DEFAULT_INBOUND_DISPATCH_TIMEOUT: Duration = Duration::from_secs(1); const DEFAULT_INBOUND_DETECT_TIMEOUT: Duration = Duration::from_secs(10); const DEFAULT_INBOUND_CONNECT_TIMEOUT: Duration = Duration::from_millis(300); -const DEFAULT_INBOUND_CONNECT_BACKOFF: ExponentialBackoff = ExponentialBackoff { - min: Duration::from_millis(100), - max: Duration::from_millis(500), - jitter: 0.1, -}; +const DEFAULT_INBOUND_CONNECT_BACKOFF: ExponentialBackoff = + ExponentialBackoff::new_unchecked(Duration::from_millis(100), Duration::from_millis(500), 0.1); const DEFAULT_OUTBOUND_DISPATCH_TIMEOUT: Duration = Duration::from_secs(3); const DEFAULT_OUTBOUND_DETECT_TIMEOUT: Duration = Duration::from_secs(10); const DEFAULT_OUTBOUND_CONNECT_TIMEOUT: Duration = Duration::from_secs(1); -const DEFAULT_OUTBOUND_CONNECT_BACKOFF: ExponentialBackoff = ExponentialBackoff { - min: Duration::from_millis(100), - max: Duration::from_millis(500), - jitter: 0.1, -}; +const DEFAULT_OUTBOUND_CONNECT_BACKOFF: ExponentialBackoff = + ExponentialBackoff::new_unchecked(Duration::from_millis(100), Duration::from_millis(500), 0.1); const DEFAULT_RESOLV_CONF: &str = "/etc/resolv.conf"; const DEFAULT_INITIAL_STREAM_WINDOW_SIZE: u32 = 65_535; // Protocol default @@ -1066,7 +1060,7 @@ pub fn parse_backoff( match (min?, max?, jitter?) { (None, None, None) => Ok(default), (Some(min), Some(max), jitter) => { - ExponentialBackoff::new(min, max, jitter.unwrap_or_default()).map_err(|error| { + ExponentialBackoff::try_new(min, max, jitter.unwrap_or_default()).map_err(|error| { error!(message="Invalid backoff", %error, %min_env, ?min, %max_env, ?max, %jitter_env, ?jitter); EnvError::InvalidEnvVar }) diff --git a/linkerd/exp-backoff/src/lib.rs b/linkerd/exp-backoff/src/lib.rs index b9088b714b..343c45c414 100644 --- a/linkerd/exp-backoff/src/lib.rs +++ b/linkerd/exp-backoff/src/lib.rs @@ -12,24 +12,22 @@ use rand::{rngs::SmallRng, thread_rng, SeedableRng}; use std::future::Future; use std::pin::Pin; use std::task::{Context, Poll}; -use std::time::Duration; use thiserror::Error; use tokio::time; /// A jittered exponential backoff strategy. -// The raw fields are exposed so this type can be constructed statically. #[derive(Copy, Clone, Debug, Default)] pub struct ExponentialBackoff { /// The minimum amount of time to wait before resuming an operation. - pub min: Duration, + min: time::Duration, /// The maximum amount of time to wait before resuming an operation. - pub max: Duration, + max: time::Duration, /// The ratio of the base timeout that may be randomly added to a backoff. /// /// Must be greater than or equal to 0.0. - pub jitter: f64, + jitter: f64, } /// A jittered exponential backoff stream. @@ -39,8 +37,8 @@ pub struct ExponentialBackoffStream { backoff: ExponentialBackoff, rng: SmallRng, iterations: u32, - sleep: Pin>, sleeping: bool, + sleep: Pin>, } #[derive(Clone, Debug, Error)] @@ -48,23 +46,19 @@ pub struct ExponentialBackoffStream { pub struct InvalidBackoff(&'static str); impl ExponentialBackoff { - pub fn stream(&self) -> ExponentialBackoffStream { - ExponentialBackoffStream { - backoff: *self, - rng: SmallRng::from_rng(&mut thread_rng()).expect("RNG must be valid"), - iterations: 0, - sleep: Box::pin(time::sleep(Duration::from_secs(0))), - sleeping: false, - } + pub const fn new_unchecked(min: time::Duration, max: time::Duration, jitter: f64) -> Self { + Self { min, max, jitter } } -} -impl ExponentialBackoff { - pub fn new(min: Duration, max: Duration, jitter: f64) -> Result { + pub fn try_new( + min: time::Duration, + max: time::Duration, + jitter: f64, + ) -> Result { if min > max { return Err(InvalidBackoff("maximum must not be less than minimum")); } - if max == Duration::from_millis(0) { + if max == time::Duration::from_millis(0) { return Err(InvalidBackoff("maximum must be non-zero")); } if jitter < 0.0 { @@ -79,13 +73,23 @@ impl ExponentialBackoff { Ok(ExponentialBackoff { min, max, jitter }) } - fn base(&self, iterations: u32) -> Duration { + pub fn stream(&self) -> ExponentialBackoffStream { + ExponentialBackoffStream { + backoff: *self, + rng: SmallRng::from_rng(&mut thread_rng()).expect("RNG must be valid"), + iterations: 0, + sleeping: false, + sleep: Box::pin(time::sleep(time::Duration::from_secs(0))), + } + } + + fn base(&self, iterations: u32) -> time::Duration { debug_assert!( self.min <= self.max, "maximum backoff must not be less than minimum backoff" ); debug_assert!( - self.max > Duration::from_millis(0), + self.max > time::Duration::from_millis(0), "Maximum backoff must be non-zero" ); self.min @@ -96,9 +100,9 @@ impl ExponentialBackoff { /// Returns a random, uniform duration on `[0, base*self.jitter]` no greater /// than `self.max`. - fn jitter(&self, base: Duration, rng: &mut R) -> Duration { + fn jitter(&self, base: time::Duration, rng: &mut R) -> time::Duration { if self.jitter == 0.0 { - Duration::default() + time::Duration::default() } else { let jitter_factor = rng.gen::(); debug_assert!( @@ -109,7 +113,7 @@ impl ExponentialBackoff { let secs = (base.as_secs() as f64) * rand_jitter; let nanos = (base.subsec_nanos() as f64) * rand_jitter; let remaining = self.max - base; - Duration::new(secs as u64, nanos as u32).min(remaining) + time::Duration::new(secs as u64, nanos as u32).min(remaining) } } } @@ -150,9 +154,9 @@ mod tests { quickcheck! { fn backoff_base_first(min_ms: u64, max_ms: u64) -> TestResult { - let min = Duration::from_millis(min_ms); - let max = Duration::from_millis(max_ms); - let backoff = match ExponentialBackoff::new(min, max, 0.0) { + let min = time::Duration::from_millis(min_ms); + let max = time::Duration::from_millis(max_ms); + let backoff = match ExponentialBackoff::try_new(min, max, 0.0) { Err(_) => return TestResult::discard(), Ok(backoff) => backoff, }; @@ -161,9 +165,9 @@ mod tests { } fn backoff_base(min_ms: u64, max_ms: u64, iterations: u32) -> TestResult { - let min = Duration::from_millis(min_ms); - let max = Duration::from_millis(max_ms); - let backoff = match ExponentialBackoff::new(min, max, 0.0) { + let min = time::Duration::from_millis(min_ms); + let max = time::Duration::from_millis(max_ms); + let backoff = match ExponentialBackoff::try_new(min, max, 0.0) { Err(_) => return TestResult::discard(), Ok(backoff) => backoff, }; @@ -172,18 +176,18 @@ mod tests { } fn backoff_jitter(base_ms: u64, max_ms: u64, jitter: f64) -> TestResult { - let base = Duration::from_millis(base_ms); - let max = Duration::from_millis(max_ms); - let backoff = match ExponentialBackoff::new(base, max, jitter) { + let base = time::Duration::from_millis(base_ms); + let max = time::Duration::from_millis(max_ms); + let backoff = match ExponentialBackoff::try_new(base, max, jitter) { Err(_) => return TestResult::discard(), Ok(backoff) => backoff, }; let j = backoff.jitter(base, &mut rand::thread_rng()); if jitter == 0.0 || base_ms == 0 || max_ms == base_ms { - TestResult::from_bool(j == Duration::default()) + TestResult::from_bool(j == time::Duration::default()) } else { - TestResult::from_bool(j > Duration::default()) + TestResult::from_bool(j > time::Duration::default()) } } }