From 8b482a8be04ed449f9d54fbaa078172900dfc71f Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Fri, 21 Jun 2024 11:19:41 +0000 Subject: [PATCH] download.rs: fix error 'captured variable cannot escape `FnMut` closure body" --- libs/utils/src/backoff.rs | 22 ++++++++--- .../tenant/remote_timeline_client/download.rs | 37 +++++++++++++++---- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/libs/utils/src/backoff.rs b/libs/utils/src/backoff.rs index 096c7e5854..9cb4b70cda 100644 --- a/libs/utils/src/backoff.rs +++ b/libs/utils/src/backoff.rs @@ -37,6 +37,20 @@ pub fn exponential_backoff_duration_seconds(n: u32, base_increment: f64, max_sec } } +pub trait Op { + async fn call(&mut self) -> Result; +} + +impl Op for F +where + F: FnMut() -> Fut, + Fut: Future>, +{ + async fn call(&mut self) -> Result { + (&mut *self)().await + } +} + /// Retries passed operation until one of the following conditions are met: /// - encountered error is considered as permanent (non-retryable) /// - retries have been exhausted @@ -51,8 +65,8 @@ pub fn exponential_backoff_duration_seconds(n: u32, base_increment: f64, max_sec /// for any other error type. Final failed attempt is logged with `{:?}`. /// /// Returns `None` if cancellation was noticed during backoff or the terminal result. -pub async fn retry( - mut op: O, +pub async fn retry( + mut op: impl Op, is_permanent: impl Fn(&E) -> bool, warn_threshold: u32, max_retries: u32, @@ -63,8 +77,6 @@ where // Not std::error::Error because anyhow::Error doesnt implement it. // For context see https://github.com/dtolnay/anyhow/issues/63 E: Display + Debug + 'static, - O: FnMut() -> F, - F: Future>, { let mut attempts = 0; loop { @@ -72,7 +84,7 @@ where return None; } - let result = op().await; + let result = op.call().await; match &result { Ok(_) => { if attempts > 0 { diff --git a/pageserver/src/tenant/remote_timeline_client/download.rs b/pageserver/src/tenant/remote_timeline_client/download.rs index cf7f0b72da..c054d1adc6 100644 --- a/pageserver/src/tenant/remote_timeline_client/download.rs +++ b/pageserver/src/tenant/remote_timeline_client/download.rs @@ -78,8 +78,33 @@ pub async fn download_layer_file<'a>( // If pageserver crashes the temp file will be deleted on startup and re-downloaded. let temp_file_path = path_with_suffix_extension(local_path, TEMP_DOWNLOAD_EXTENSION); + struct DownloadObjectClosure<'a> { + storage: &'a GenericRemoteStorage, + remote_path: &'a RemotePath, + temp_file_path: &'a Utf8PathBuf, + cancel: &'a CancellationToken, + ctx: &'a mut RequestContext, + } + impl backoff::Op for DownloadObjectClosure<'_> { + async fn call(&mut self) -> Result { + let DownloadObjectClosure { + storage, + remote_path, + temp_file_path, + cancel, + ctx, + } = self; + download_object(storage, remote_path, temp_file_path, cancel, ctx).await + } + } let bytes_amount = download_retry( - || async { download_object(storage, &remote_path, &temp_file_path, cancel, ctx).await }, + DownloadObjectClosure { + storage, + remote_path: &remote_path, + temp_file_path: &temp_file_path, + cancel, + ctx, + }, &format!("download {remote_path:?}"), cancel, ) @@ -568,15 +593,11 @@ pub(crate) async fn download_initdb_tar_zst( /// with backoff. /// /// (See similar logic for uploads in `perform_upload_task`) -pub(super) async fn download_retry( - op: O, +pub(super) async fn download_retry( + op: impl backoff::Op, description: &str, cancel: &CancellationToken, -) -> Result -where - O: FnMut() -> F, - F: Future>, -{ +) -> Result { backoff::retry( op, DownloadError::is_permanent,