From a57d02b615424fb1b9e7b5c33a2d14b932c2c390 Mon Sep 17 00:00:00 2001 From: Alexander Weiss Date: Fri, 30 Jun 2023 16:21:01 +0200 Subject: [PATCH] rest/rclone: make # of retries cusomizable and use sensible default --- changelog/new.txt | 1 + config/full.toml | 2 +- crates/rustic_core/src/backend/rest.rs | 62 +++++++++++++++----------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/changelog/new.txt b/changelog/new.txt index 7211335c3..f60a6c6e0 100644 --- 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 index f0181f42a..8f692ca7e 100644 --- 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 = "5" # Only rest/rclone backend; Allowed values: "false" 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 index 6e963de4a..740200376 100644 --- a/crates/rustic_core/src/backend/rest.rs +++ b/crates/rustic_core/src/backend/rest.rs @@ -38,19 +38,37 @@ impl CheckError for Response { } #[derive(Clone, Debug)] -struct MaybeBackoff(Option); +struct LimitRetryBackoff { + max_retries: usize, + retries: usize, + exp: ExponentialBackoff, +} + +impl Default for LimitRetryBackoff { + fn default() -> Self { + Self { + max_retries: 0, + 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 { - 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(); } } @@ -58,7 +76,7 @@ impl Backoff for MaybeBackoff { pub struct RestBackend { url: Url, client: Client, - backoff: MaybeBackoff, + backoff: LimitRetryBackoff, } fn notify(err: reqwest::Error, duration: Duration) { @@ -88,11 +106,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 +140,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 = if value == "false" { + 0 + } else { + 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,