Merge branch 'simplify-backoff'

3 files changed, 49 insertions, 54 deletions

diff --git a/mullvad-daemon/src/relays.rs b/mullvad-daemon/src/relays.rs
index 2e2a009c4d..17906cc686 100644
--- a/mullvad-daemon/src/relays.rs
+++ b/mullvad-daemon/src/relays.rs
@@ -47,9 +47,8 @@ const UPDATE_CHECK_INTERVAL: Duration = Duration::from_secs(60 * 15);
 /// How old the cached relays need to be to trigger an update
 const UPDATE_INTERVAL: Duration = Duration::from_secs(60 * 60);
 
-/// First delay of exponential backoff in milliseconds
-const EXPONENTIAL_BACKOFF_DELAY_MS: u64 = 30;
-const EXPONENTIAL_BACKOFF_FACTOR: u64 = 2000;
+const EXPONENTIAL_BACKOFF_INITIAL: Duration = Duration::from_secs(16);
+const EXPONENTIAL_BACKOFF_FACTOR: u32 = 8;
 
 #[derive(err_derive::Error, Debug)]
 #[error(no_from)]
@@ -887,9 +886,9 @@ impl RelayListUpdater {
     ) -> impl Future<Output = Result<Option<RelayList>, mullvad_rpc::rest::Error>> + 'static {
         let download_futures = move || rpc_handle.relay_list(tag.clone());
 
-        let exponential_backoff = ExponentialBackoff::from_millis(EXPONENTIAL_BACKOFF_DELAY_MS)
-            .factor(EXPONENTIAL_BACKOFF_FACTOR)
-            .max_delay(UPDATE_INTERVAL * 2);
+        let exponential_backoff =
+            ExponentialBackoff::new(EXPONENTIAL_BACKOFF_INITIAL, EXPONENTIAL_BACKOFF_FACTOR)
+                .max_delay(UPDATE_INTERVAL * 2);
 
         let download_future = retry_future_with_backoff(
             download_futures,
diff --git a/mullvad-daemon/src/wireguard.rs b/mullvad-daemon/src/wireguard.rs
index 0939a0afac..3fa39a1455 100644
--- a/mullvad-daemon/src/wireguard.rs
+++ b/mullvad-daemon/src/wireguard.rs
@@ -34,6 +34,10 @@ const ROTATION_START_DELAY: Duration = Duration::from_secs(60 * 3);
 /// A short interval is used in case the computer is ever suspended.
 const KEY_CHECK_INTERVAL: Duration = Duration::from_secs(60);
 
+const RETRY_KEY_PUSH_INITIAL: Duration = Duration::from_millis(300);
+const RETRY_KEY_PUSH_FACTOR: u32 = 300;
+const RETRY_KEY_PUSH_MAX: Duration = Duration::from_secs(60 * 60);
+
 #[derive(err_derive::Error, Debug)]
 pub enum Error {
     #[error(display = "Unexpected HTTP request error")]
@@ -194,7 +198,8 @@ impl KeyManager {
 
 
         let retry_strategy = Jittered::jitter(
-            ExponentialBackoff::from_millis(300).max_delay(Duration::from_secs(60 * 60)),
+            ExponentialBackoff::new(RETRY_KEY_PUSH_INITIAL, RETRY_KEY_PUSH_FACTOR)
+                .max_delay(RETRY_KEY_PUSH_MAX),
         );
 
         let should_retry = move |result: &std::result::Result<_, bool>| -> bool {
diff --git a/talpid-core/src/future_retry.rs b/talpid-core/src/future_retry.rs
index 8b4745a7b2..2a7a08a092 100644
--- a/talpid-core/src/future_retry.rs
+++ b/talpid-core/src/future_retry.rs
@@ -40,32 +40,24 @@ async fn sleep(mut delay: Duration) {
 
 /// Provides an exponential back-off timer to delay the next retry of a failed operation.
 pub struct ExponentialBackoff {
-    current: u64,
-    base: u64,
-    factor: u64,
+    next: Duration,
+    factor: u32,
     max_delay: Option<Duration>,
 }
 
 impl ExponentialBackoff {
-    /// Creates a `ExponentialBackoff` with the provided number of milliseconds as a base.
+    /// Creates a `ExponentialBackoff` starting with the provided duration.
     ///
-    /// All else staying the same, the first delay will be `millis` milliseconds long, the second
-    /// one will be `millis^2`, third `millis^3` and so on.
-    pub fn from_millis(millis: u64) -> ExponentialBackoff {
+    /// All else staying the same, the first delay will be `initial` long, the second
+    /// one will be `initial * factor`, third `initial * factor^2` and so on.
+    pub fn new(initial: Duration, factor: u32) -> ExponentialBackoff {
         ExponentialBackoff {
-            current: millis,
-            base: millis,
-            factor: 1u64,
+            next: initial,
+            factor,
             max_delay: None,
         }
     }
 
-    /// Sets the constant factor of the delays. The default value is 1.
-    pub fn factor(mut self, factor: u64) -> ExponentialBackoff {
-        self.factor = factor;
-        self
-    }
-
     /// Set the maximum delay. By default, there is no maximum value set, but the practical limit
     /// is `std::u64::MAX`.
     pub fn max_delay(mut self, duration: Duration) -> ExponentialBackoff {
@@ -75,25 +67,20 @@ impl ExponentialBackoff {
 
     /// Returns the value of the delay and advances the next back-off delay.
     fn next_delay(&mut self) -> Duration {
-        let delay_msec = self
-            .current
-            .checked_mul(self.factor)
-            .unwrap_or(std::u64::MAX);
-        let delay = Duration::from_millis(delay_msec);
+        let next = self.next;
 
         if let Some(max_delay) = self.max_delay {
-            if delay > max_delay {
+            if next > max_delay {
                 return max_delay;
             }
         }
 
-        self.current = self.current.checked_mul(self.base).unwrap_or(std::u64::MAX);
-        delay
-    }
+        // TODO: Use Duration::MAX when it is available
+        self.next = next
+            .checked_mul(self.factor)
+            .unwrap_or(Duration::from_secs(u64::MAX));
 
-    /// Resets the delay to it's initial state.
-    pub fn reset(&mut self) {
-        self.current = self.base;
+        next
     }
 }
 
@@ -141,43 +128,47 @@ mod test {
     use super::*;
 
     #[test]
-    fn test_exponetnial_backoff() {
-        let mut backoff = ExponentialBackoff::from_millis(2).factor(1000);
+    fn test_exponential_backoff() {
+        let mut backoff = ExponentialBackoff::new(Duration::from_secs(2), 3);
 
         assert_eq!(backoff.next(), Some(Duration::from_secs(2)));
-        assert_eq!(backoff.next(), Some(Duration::from_secs(4)));
-        assert_eq!(backoff.next(), Some(Duration::from_secs(8)));
-        backoff.reset();
-        assert_eq!(backoff.next(), Some(Duration::from_secs(2)));
+        assert_eq!(backoff.next(), Some(Duration::from_secs(6)));
+        assert_eq!(backoff.next(), Some(Duration::from_secs(18)));
     }
 
     #[test]
     fn test_at_maximum_value() {
-        let mut backoff = ExponentialBackoff::from_millis(std::u64::MAX - 1);
+        let max = Duration::from_secs(u64::MAX);
+        let mu = Duration::from_micros(1);
+        let mut backoff = ExponentialBackoff::new(max - mu, 2);
 
-        assert_eq!(
-            backoff.next(),
-            Some(Duration::from_millis(std::u64::MAX - 1))
-        );
-        assert_eq!(backoff.next(), Some(Duration::from_millis(std::u64::MAX)));
-        assert_eq!(backoff.next(), Some(Duration::from_millis(std::u64::MAX)));
+        assert_eq!(backoff.next(), Some(max - mu));
+        assert_eq!(backoff.next(), Some(max));
+        assert_eq!(backoff.next(), Some(max));
     }
 
     #[test]
     fn test_maximum_bound() {
-        let mut backoff = ExponentialBackoff::from_millis(2).max_delay(Duration::from_millis(4));
+        let mut backoff = ExponentialBackoff::new(Duration::from_millis(2), 3)
+            .max_delay(Duration::from_millis(7));
 
         assert_eq!(backoff.next(), Some(Duration::from_millis(2)));
-        assert_eq!(backoff.next(), Some(Duration::from_millis(4)));
-        assert_eq!(backoff.next(), Some(Duration::from_millis(4)));
+        assert_eq!(backoff.next(), Some(Duration::from_millis(6)));
+        assert_eq!(backoff.next(), Some(Duration::from_millis(7)));
     }
 
     #[test]
     fn test_minimum_value() {
-        let mut backoff = ExponentialBackoff::from_millis(0);
+        let zero = Duration::from_millis(0);
+        let mut backoff = ExponentialBackoff::new(zero, 10);
+
+        assert_eq!(backoff.next(), Some(zero));
+        assert_eq!(backoff.next(), Some(zero));
+
+        let mut backoff = ExponentialBackoff::new(Duration::from_millis(1), 0);
 
-        assert_eq!(backoff.next(), Some(Duration::from_millis(0)));
-        assert_eq!(backoff.next(), Some(Duration::from_millis(0)));
+        assert_eq!(backoff.next(), Some(Duration::from_millis(1)));
+        assert_eq!(backoff.next(), Some(zero));
     }
 
     #[test]