dist: don't count download attempts that make progress as retries

Author: djc

Cargo.lock

@@ -92,7 +92,6 @@ tempfile = "3.8"
 thiserror = "2"
 threadpool = "1"
 tokio = { version = "1.26.0", default-features = false, features = ["macros", "rt-multi-thread", "sync"] }
-tokio-retry = "0.3.0"
 tokio-stream = "0.1.14"
 toml = "1.0"
 tracing = "0.1"

Cargo.toml

@@ -52,7 +52,9 @@ should one arise with the backtrack limits.
 When downloading a file, rustup will retry the download a number of times. The
 default is 3 times, but if this variable is set to a valid usize then it is the
 max retry count. A value of `0` means no retries, thus the default of `3` will
-mean a download is tried a total of four times before failing out.
+mean a download is tried a total of four times before failing out. Attempts
+that download at least 1 MiB of the file before failing are resumed by the
+next attempt and don't count towards this limit.
 
 ## `RUSTUP_BACKTRACE`
 

doc/dev-guide/src/tips-and-tricks.md

@@ -77,15 +77,7 @@ impl<'a> DownloadCfg<'a> {
             }
         }
 
-        let partial_file_path = target_file.with_file_name(
-            target_file
-                .file_name()
-                .map(|s| s.to_str().unwrap_or("_"))
-                .unwrap_or("_")
-                .to_owned()
-                + ".partial",
-        );
-
+        let partial_file_path = self.partial_file_path(hash);
         let partial_file_existed = partial_file_path.exists();
 
         let mut hasher = Sha256::new();
@@ -132,6 +124,18 @@ impl<'a> DownloadCfg<'a> {
         }
     }
 
+    /// The number of bytes already downloaded for the file with the given hash
+    pub(crate) fn partial_len(&self, hash: &str) -> u64 {
+        match fs::metadata(self.partial_file_path(hash)) {
+            Ok(metadata) => metadata.len(),
+            Err(_) => 0,
+        }
+    }
+
+    fn partial_file_path(&self, hash: &str) -> PathBuf {
+        self.download_dir.join(format!("{hash}.partial"))
+    }
+
     pub(crate) fn clean(&self, hashes: &[impl AsRef<Path>]) -> anyhow::Result<()> {
         for hash in hashes.iter() {
             let used_file = self.download_dir.join(hash);

src/dist/download.rs

@@ -20,6 +20,7 @@ use futures_util::{
 };
 use tokio::task::{JoinHandle, spawn_blocking};
 use tracing::{debug, info, warn};
+use url::Url;
 
 use crate::{
     diskio::{Executor, IO_CHUNK_SIZE, get_executor, unpack_ram},
@@ -771,32 +772,13 @@ impl<'a> ComponentBinary<'a> {
     }
 
     async fn download(self, max_retries: usize) -> anyhow::Result<(ComponentInstall, &'a str)> {
-        use tokio_retry::{RetryIf, strategy::FixedInterval};
-
         let url = self.download_cfg.url(&self.binary.url)?;
-        let installer = RetryIf::start(
-            FixedInterval::from_millis(0).take(max_retries),
-            || {
-                self.download_cfg
-                    .download(&url, &self.binary.hash, &self.status)
-            },
-            |e: &anyhow::Error| {
-                // retry only known retriable cases
-                match e.downcast_ref::<RustupError>() {
-                    Some(RustupError::BrokenPartialFile)
-                    | Some(RustupError::IncompletePartialFile)
-                    | Some(RustupError::DownloadingFile { .. }) => {
-                        self.status.retrying();
-                        true
-                    }
-                    _ => false,
-                }
-            },
-        )
-        .await
-        .with_context(|| {
-            RustupError::ComponentDownloadFailed(self.manifest.name(&self.component))
-        })?;
+        let installer = self
+            .download_with_retries(&url, max_retries)
+            .await
+            .with_context(|| {
+                RustupError::ComponentDownloadFailed(self.manifest.name(&self.component))
+            })?;
 
         let install = ComponentInstall {
             status: self.status,
@@ -813,6 +795,44 @@ impl<'a> ComponentBinary<'a> {
 
         Ok((install, &self.binary.hash))
     }
+
+    async fn download_with_retries(&self, url: &Url, max_retries: usize) -> anyhow::Result<File> {
+        const MIN_PROGRESS: u64 = 1024 * 1024;
+
+        let mut retries = 0;
+        loop {
+            let downloaded = self.download_cfg.partial_len(&self.binary.hash);
+            let err = match self
+                .download_cfg
+                .download(url, &self.binary.hash, &self.status)
+                .await
+            {
+                Ok(file) => return Ok(file),
+                Err(err) => err,
+            };
+
+            // retry only known retriable cases
+            match err.downcast_ref::<RustupError>() {
+                Some(RustupError::BrokenPartialFile)
+                | Some(RustupError::IncompletePartialFile)
+                | Some(RustupError::DownloadingFile { .. }) => {}
+                _ => return Err(err),
+            }
+
+            let progress = self
+                .download_cfg
+                .partial_len(&self.binary.hash)
+                .saturating_sub(downloaded);
+            if progress < MIN_PROGRESS {
+                if retries == max_retries {
+                    return Err(err);
+                }
+                retries += 1;
+            }
+
+            self.status.retrying();
+        }
+    }
 }
 
 struct ComponentInstall {