From 38c89e232c4ed3f06282fac9d40bc3f37f8da7b4 Mon Sep 17 00:00:00 2001 From: Seva Zaikov Date: Sat, 11 Jul 2026 22:39:54 -0700 Subject: [PATCH] update retry config with builder api --- examples/custom_retry.rs | 15 ++--- src/data_reader.rs | 5 +- src/read_loop.rs | 25 ++++----- src/retry.rs | 115 +++++++++++++++++++++++++++++++-------- src/stream.rs | 5 +- 5 files changed, 112 insertions(+), 53 deletions(-) diff --git a/examples/custom_retry.rs b/examples/custom_retry.rs index 78646e2..a52cbe7 100644 --- a/examples/custom_retry.rs +++ b/examples/custom_retry.rs @@ -3,6 +3,8 @@ /// By default, it already retries multiple times with smaller number /// of sectors, so this usually should not be necessary, but you can see /// here that you can tweak details. +use std::time::Duration; + use cd_da_reader::{CdReader, ReadOptions, RetryConfig}; fn main() -> Result<(), Box> { @@ -17,13 +19,12 @@ fn main() -> Result<(), Box> { // More attempts, longer backoff, and sector reduction down to 1 // for maximum resilience on scratched media. - let retry = RetryConfig { - max_attempts: 8, - initial_backoff_ms: 50, - max_backoff_ms: 1000, - reduce_chunk_on_retry: true, - min_sectors_per_read: 1, - }; + let retry = RetryConfig::default() + .with_max_attempts(8) + .with_initial_backoff(Duration::from_millis(50)) + .with_max_backoff(Duration::from_secs(1)) + .with_chunk_reduction(true) + .with_min_sectors_per_read(1); let options = ReadOptions::default().with_retry(retry); println!( diff --git a/src/data_reader.rs b/src/data_reader.rs index 25d20fa..a229740 100644 --- a/src/data_reader.rs +++ b/src/data_reader.rs @@ -130,10 +130,7 @@ mod tests { fn read_options_builders_override_individual_defaults() { assert_eq!(ReadOptions::default().mode(), SectorReadMode::Audio); - let retry = crate::RetryConfig { - max_attempts: 9, - ..crate::RetryConfig::default() - }; + let retry = crate::RetryConfig::default().with_max_attempts(9); let options = ReadOptions::default() .with_mode(SectorReadMode::DataRaw) .with_retry(retry); diff --git a/src/read_loop.rs b/src/read_loop.rs index f2e1bb8..72d110c 100644 --- a/src/read_loop.rs +++ b/src/read_loop.rs @@ -7,7 +7,6 @@ //! macOS) is platform-specific, so it is injected as a closure. use std::thread::sleep; -use std::time::Duration; use crate::data_reader::SectorReadMode; use crate::{CdReaderError, RetryConfig}; @@ -47,7 +46,7 @@ where while remaining > 0 { let mut chunk_sectors = remaining.min(max_sectors_per_xfer); - let mut backoff_ms = cfg.initial_backoff_ms; + let mut backoff = cfg.initial_backoff.min(cfg.max_backoff); let mut last_err: Option = None; for attempt in 1..=attempts_total { @@ -83,12 +82,10 @@ where if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); } - if backoff_ms > 0 { - sleep(Duration::from_millis(backoff_ms)); - } - if cfg.max_backoff_ms > 0 { - backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); + if !backoff.is_zero() { + sleep(backoff); } + backoff = backoff.saturating_mul(2).min(cfg.max_backoff); } } } @@ -120,17 +117,17 @@ fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { #[cfg(test)] mod tests { + use std::time::Duration; + use super::read_sectors_chunked; use crate::{CdReaderError, RetryConfig, SectorReadMode}; fn retry_config(max_attempts: u8, reduce_chunk_on_retry: bool) -> RetryConfig { - RetryConfig { - max_attempts, - initial_backoff_ms: 0, - max_backoff_ms: 0, - reduce_chunk_on_retry, - min_sectors_per_read: 1, - } + RetryConfig::default() + .with_max_attempts(max_attempts) + .with_initial_backoff(Duration::ZERO) + .with_max_backoff(Duration::ZERO) + .with_chunk_reduction(reduce_chunk_on_retry) } #[test] diff --git a/src/retry.rs b/src/retry.rs index ee5f1f5..541c5b7 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -1,42 +1,109 @@ -/// Retry policy for idempotent read operations. +use std::time::Duration; + +/// Retry policy for read operations. +/// +/// The policy is applied when we fail to read a chunk, and it will both +/// wait a bit before attempting the next read and will decrease the number +/// of chunks to read. The default values are aimed to be universally good +/// and unless you have specific requirements using RetryConfig::default() +/// is recommended. +/// +/// The default policy uses: /// -/// The policy is applied per failed read chunk/command and can combine -/// capped exponential backoff with adaptive chunk-size reduction. +/// - 4 attempts, including the initial read; +/// - a 20 ms initial backoff; +/// - a 300 ms maximum backoff; +/// - adaptive chunk reduction; +/// - a minimum chunk size of 1 sector. #[derive(Debug, Clone)] pub struct RetryConfig { - /// Maximum attempts per operation, including the initial attempt. - /// By default the value is 4. + pub(crate) max_attempts: u8, + pub(crate) initial_backoff: Duration, + pub(crate) max_backoff: Duration, + pub(crate) reduce_chunk_on_retry: bool, + pub(crate) min_sectors_per_read: u32, +} + +impl RetryConfig { + /// Set the maximum attempts per operation, including the initial attempt. /// - /// Values below `1` are normalized by callers to at least one attempt. - pub max_attempts: u8, - /// Initial backoff delay in milliseconds before the second attempt. - /// First attempt is always immediate, so if there are no issues during - /// reading, we don't wait any time. - pub initial_backoff_ms: u64, - /// Upper bound for exponential backoff delay in milliseconds. + /// A value of zero is normalized to one attempt. + pub fn with_max_attempts(mut self, attempts: u8) -> Self { + self.max_attempts = attempts.max(1); + self + } + + /// Set the delay before the second attempt. /// - /// Each retry typically doubles the previous delay until this cap is reached. - pub max_backoff_ms: u64, - /// Enable adaptive sector-count reduction on retry for `READ CD` operations. + /// The first attempt is always immediate. + pub fn with_initial_backoff(mut self, backoff: Duration) -> Self { + self.initial_backoff = backoff; + self + } + + /// Set the upper bound for exponential backoff delays. /// - /// Current implementation reduces chunk size from large reads toward smaller - /// reads (for example `27 -> 8 -> 1`) to have a higher change of success. - pub reduce_chunk_on_retry: bool, - /// Minimum sectors per `READ CD` command when adaptive reduction is enabled. - /// Default value is 27 for 64KB per read. + /// A duration of zero disables retry delays. + pub fn with_max_backoff(mut self, backoff: Duration) -> Self { + self.max_backoff = backoff; + self + } + + /// Enable or disable adaptive sector-count reduction after failed reads. + pub fn with_chunk_reduction(mut self, enabled: bool) -> Self { + self.reduce_chunk_on_retry = enabled; + self + } + + /// Set the minimum sectors per command when adaptive reduction is enabled. /// - /// Use `1` for maximal fault isolation; larger values can improve throughput. - pub min_sectors_per_read: u32, + /// A value of zero is normalized to one sector. + pub fn with_min_sectors_per_read(mut self, sectors: u32) -> Self { + self.min_sectors_per_read = sectors.max(1); + self + } } impl Default for RetryConfig { fn default() -> Self { Self { max_attempts: 4, - initial_backoff_ms: 20, - max_backoff_ms: 300, + initial_backoff: Duration::from_millis(20), + max_backoff: Duration::from_millis(300), reduce_chunk_on_retry: true, min_sectors_per_read: 1, } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_policy_matches_documented_values() { + let config = RetryConfig::default(); + + assert_eq!(config.max_attempts, 4); + assert_eq!(config.initial_backoff, Duration::from_millis(20)); + assert_eq!(config.max_backoff, Duration::from_millis(300)); + assert!(config.reduce_chunk_on_retry); + assert_eq!(config.min_sectors_per_read, 1); + } + + #[test] + fn builders_override_and_normalize_values() { + let config = RetryConfig::default() + .with_max_attempts(0) + .with_initial_backoff(Duration::from_millis(50)) + .with_max_backoff(Duration::from_secs(1)) + .with_chunk_reduction(false) + .with_min_sectors_per_read(0); + + assert_eq!(config.max_attempts, 1); + assert_eq!(config.initial_backoff, Duration::from_millis(50)); + assert_eq!(config.max_backoff, Duration::from_secs(1)); + assert!(!config.reduce_chunk_on_retry); + assert_eq!(config.min_sectors_per_read, 1); + } +} diff --git a/src/stream.rs b/src/stream.rs index 9e8dab8..299e165 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -216,10 +216,7 @@ mod tests { #[test] fn options_builders_override_individual_defaults() { - let retry = RetryConfig { - max_attempts: 9, - ..RetryConfig::default() - }; + let retry = RetryConfig::default().with_max_attempts(9); let options = TrackStreamOptions::default() .with_mode(SectorReadMode::DataRaw) .with_retry(retry)