Merge pull request #720 from rustic-rs/retries

rest/rclone: make # of retries cusomizable and use sensible default
rustic-rs · Jul 17, 2023 · 35f0cb0 · 35f0cb0
2 parents 1232a06 + 5bd4fa5
commit 35f0cb0
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 28 deletions.
diff --git a/changelog/new.txt b/changelog/new.txt
@@ -4,6 +4,7 @@ Breaking changes:
 
 Bugs fixed:
 - prune did abort when no time was set for a pack-do-delete. This case is now handled correctly.
+- retrying backend access didn't work for long operations. This has been fixed (and retries are now customizable)
 
 New features:
 - New global configuration paths are available, located at /etc/rustic/*.toml or %PROGRAMDATA%/rustic/config/*.toml, depending on your platform.

diff --git a/config/full.toml b/config/full.toml
@@ -33,7 +33,7 @@ warm-up-wait = "10min" # Default: not set
 [repository.options]
 post-create-command = "par2create -qq -n1 -r5 %file" # Only local backend; Default: not set
 post-delete-command = "sh -c \"rm -f %file*.par2\"" # Only local backend; Default: not set 
-retry = "true" # Only rest/rclone backend
+retry = "default" # Only rest/rclone backend; Allowed values: "false"/"off", "default" or number of retries
 timeout = "10min" # Only rest/rclone backend
 
 # Snapshot-filter options: These options apply to all commands that use snapshot filters

diff --git a/crates/rustic_core/src/backend/rest.rs b/crates/rustic_core/src/backend/rest.rs
@@ -17,6 +17,10 @@ use crate::{
     id::Id,
 };
 
+mod consts {
+    pub(super) const DEFAULT_RETRY: usize = 5;
+}
+
 // trait CheckError to add user-defined method check_error on Response
 pub(crate) trait CheckError {
     fn check_error(self) -> Result<Response, Error<reqwest::Error>>;
@@ -38,27 +42,45 @@ impl CheckError for Response {
 }
 
 #[derive(Clone, Debug)]
-struct MaybeBackoff(Option<ExponentialBackoff>);
+struct LimitRetryBackoff {
+    max_retries: usize,
+    retries: usize,
+    exp: ExponentialBackoff,
+}
+
+impl Default for LimitRetryBackoff {
+    fn default() -> Self {
+        Self {
+            max_retries: consts::DEFAULT_RETRY,
+            retries: 0,
+            exp: ExponentialBackoffBuilder::new()
+                .with_max_elapsed_time(None) // no maximum elapsed time; we count number of retires
+                .build(),
+        }
+    }
+}
 
-impl Backoff for MaybeBackoff {
+impl Backoff for LimitRetryBackoff {
     fn next_backoff(&mut self) -> Option<Duration> {
-        self.0
-            .as_mut()
-            .and_then(backoff::backoff::Backoff::next_backoff)
+        self.retries += 1;
+        if self.retries > self.max_retries {
+            None
+        } else {
+            self.exp.next_backoff()
+        }
     }
 
     fn reset(&mut self) {
-        if let Some(b) = self.0.as_mut() {
-            b.reset();
-        }
+        self.retries = 0;
+        self.exp.reset();
     }
 }
 
 #[derive(Clone, Debug)]
 pub struct RestBackend {
     url: Url,
     client: Client,
-    backoff: MaybeBackoff,
+    backoff: LimitRetryBackoff,
 }
 
 fn notify(err: reqwest::Error, duration: Duration) {
@@ -88,11 +110,7 @@ impl RestBackend {
         Ok(Self {
             url,
             client,
-            backoff: MaybeBackoff(Some(
-                ExponentialBackoffBuilder::new()
-                    .with_max_elapsed_time(Some(Duration::from_secs(600)))
-                    .build(),
-            )),
+            backoff: LimitRetryBackoff::default(),
         })
     }
 
@@ -126,19 +144,13 @@ impl ReadBackend for RestBackend {
 
     fn set_option(&mut self, option: &str, value: &str) -> RusticResult<()> {
         if option == "retry" {
-            match value {
-                "true" => {
-                    self.backoff = MaybeBackoff(Some(
-                        ExponentialBackoffBuilder::new()
-                            .with_max_elapsed_time(Some(Duration::from_secs(120)))
-                            .build(),
-                    ));
-                }
-                "false" => {
-                    self.backoff = MaybeBackoff(None);
-                }
-                val => return Err(RestErrorKind::NotSupportedForRetry(val.into()).into()),
-            }
+            let max_retries = match value {
+                "false" | "off" => 0,
+                "default" => consts::DEFAULT_RETRY,
+                _ => usize::from_str(value)
+                    .map_err(|_| RestErrorKind::NotSupportedForRetry(value.into()))?,
+            };
+            self.backoff.max_retries = max_retries;
         } else if option == "timeout" {
             let timeout = match humantime::Duration::from_str(value) {
                 Ok(val) => val,