diff --git a/crates/common/src/resilience/retry.rs b/crates/common/src/resilience/retry.rs index 35492f514..585a4dbe6 100644 --- a/crates/common/src/resilience/retry.rs +++ b/crates/common/src/resilience/retry.rs @@ -44,6 +44,8 @@ pub struct RetryConfig { pub base_delay: Duration, /// Maximum delay cap for exponential backoff pub max_delay: Duration, + /// Backoff multiplier for exponential delay calculation (default 2.0) + pub backoff_multiplier: f64, /// Circuit breaker failure threshold pub circuit_breaker_threshold: u32, /// Circuit breaker timeout before reset attempt @@ -60,6 +62,7 @@ impl Default for RetryConfig { max_retries: 3, base_delay: Duration::from_millis(100), max_delay: Duration::from_secs(10), + backoff_multiplier: 2.0, circuit_breaker_threshold: 5, circuit_breaker_timeout: Duration::from_secs(60), enable_jitter: true, @@ -75,6 +78,7 @@ impl RetryConfig { max_retries: 3, base_delay: Duration::from_micros(50), // 50µs base delay max_delay: Duration::from_millis(5), // 5ms max delay + backoff_multiplier: 2.0, circuit_breaker_threshold: 10, circuit_breaker_timeout: Duration::from_secs(30), enable_jitter: false, // No jitter for HFT - predictable timing @@ -88,6 +92,7 @@ impl RetryConfig { max_retries: 5, base_delay: Duration::from_millis(500), max_delay: Duration::from_secs(10), + backoff_multiplier: 2.0, circuit_breaker_threshold: 3, circuit_breaker_timeout: Duration::from_secs(30), enable_jitter: true, @@ -101,6 +106,7 @@ impl RetryConfig { max_retries: 7, base_delay: Duration::from_millis(250), max_delay: Duration::from_secs(5), + backoff_multiplier: 2.0, circuit_breaker_threshold: 5, circuit_breaker_timeout: Duration::from_secs(45), enable_jitter: true, diff --git a/crates/storage/src/model_helpers.rs b/crates/storage/src/model_helpers.rs index 39f38be02..4b75ceaa9 100644 --- a/crates/storage/src/model_helpers.rs +++ b/crates/storage/src/model_helpers.rs @@ -136,29 +136,8 @@ pub struct EnhancedObjectStoreBackend { _retry_config: RetryConfig, } -/// Retry configuration for robust operations -#[derive(Debug, Clone)] -pub struct RetryConfig { - /// Maximum number of retry attempts - pub max_attempts: u32, - /// Initial backoff delay - pub initial_delay: Duration, - /// Maximum backoff delay - pub max_delay: Duration, - /// Backoff multiplier - pub backoff_multiplier: f64, -} - -impl Default for RetryConfig { - fn default() -> Self { - Self { - max_attempts: 3, - initial_delay: Duration::from_millis(100), - max_delay: Duration::from_secs(30), - backoff_multiplier: 2.0, - } - } -} +/// Re-export `RetryConfig` from common for storage consumers +pub use common::resilience::retry::RetryConfig; impl EnhancedObjectStoreBackend { /// Create new enhanced backend with connection pooling diff --git a/crates/storage/src/object_store_backend.rs b/crates/storage/src/object_store_backend.rs index ef14a1dfa..b01cc2026 100644 --- a/crates/storage/src/object_store_backend.rs +++ b/crates/storage/src/object_store_backend.rs @@ -134,15 +134,15 @@ impl ObjectStoreBackend { F: Fn() -> Fut, Fut: std::future::Future>, { - let mut delay = self.retry_config.initial_delay; + let mut delay = self.retry_config.base_delay; let mut last_error = None; - for attempt in 1..=self.retry_config.max_attempts { + for attempt in 1..=self.retry_config.max_retries { match operation().await { Ok(result) => return Ok(result), Err(e) => { last_error = Some(e); - if attempt < self.retry_config.max_attempts { + if attempt < self.retry_config.max_retries { debug!( "Operation failed on attempt {}, retrying in {:?}", attempt, delay diff --git a/crates/storage/tests/model_helpers_tests.rs b/crates/storage/tests/model_helpers_tests.rs index 4d7860af5..a9daf6ab7 100644 --- a/crates/storage/tests/model_helpers_tests.rs +++ b/crates/storage/tests/model_helpers_tests.rs @@ -166,23 +166,24 @@ fn test_model_info_no_versions() { fn test_retry_config_default() { let config = RetryConfig::default(); - assert_eq!(config.max_attempts, 3); - assert_eq!(config.initial_delay, std::time::Duration::from_millis(100)); - assert_eq!(config.max_delay, std::time::Duration::from_secs(30)); // Actual default is 30s + assert_eq!(config.max_retries, 3); + assert_eq!(config.base_delay, std::time::Duration::from_millis(100)); + assert_eq!(config.max_delay, std::time::Duration::from_secs(10)); assert_eq!(config.backoff_multiplier, 2.0); } #[test] fn test_retry_config_custom() { let config = RetryConfig { - max_attempts: 5, - initial_delay: std::time::Duration::from_millis(50), + max_retries: 5, + base_delay: std::time::Duration::from_millis(50), max_delay: std::time::Duration::from_secs(5), backoff_multiplier: 1.5, + ..Default::default() }; - assert_eq!(config.max_attempts, 5); - assert_eq!(config.initial_delay, std::time::Duration::from_millis(50)); + assert_eq!(config.max_retries, 5); + assert_eq!(config.base_delay, std::time::Duration::from_millis(50)); assert_eq!(config.max_delay, std::time::Duration::from_secs(5)); assert_eq!(config.backoff_multiplier, 1.5); } diff --git a/crates/storage/tests/network_edge_cases_tests.rs b/crates/storage/tests/network_edge_cases_tests.rs index 84e865f82..4609c0621 100644 --- a/crates/storage/tests/network_edge_cases_tests.rs +++ b/crates/storage/tests/network_edge_cases_tests.rs @@ -31,10 +31,11 @@ async fn test_network_timeout_handling() { // Configure aggressive retry with short timeouts let retry_config = RetryConfig { - max_attempts: 2, - initial_delay: std::time::Duration::from_millis(10), + max_retries: 2, + base_delay: std::time::Duration::from_millis(10), max_delay: std::time::Duration::from_millis(50), backoff_multiplier: 2.0, + ..Default::default() }; let backend = backend.with_retry_config(retry_config); diff --git a/crates/storage/tests/object_store_backend_tests.rs b/crates/storage/tests/object_store_backend_tests.rs index 90f73ba3b..201f34f85 100644 --- a/crates/storage/tests/object_store_backend_tests.rs +++ b/crates/storage/tests/object_store_backend_tests.rs @@ -180,10 +180,11 @@ async fn test_with_retry_config() { let backend = create_test_backend(); let retry_config = RetryConfig { - max_attempts: 5, - initial_delay: std::time::Duration::from_millis(50), + max_retries: 5, + base_delay: std::time::Duration::from_millis(50), max_delay: std::time::Duration::from_secs(1), backoff_multiplier: 1.5, + ..Default::default() }; let backend = backend.with_retry_config(retry_config); diff --git a/crates/storage/tests/s3_tests.rs b/crates/storage/tests/s3_tests.rs index a2656afe6..6aaa85606 100644 --- a/crates/storage/tests/s3_tests.rs +++ b/crates/storage/tests/s3_tests.rs @@ -238,10 +238,11 @@ async fn test_upload_retry_transient_failures() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); let test_data = b"test data for retry"; @@ -266,10 +267,11 @@ async fn test_upload_failure_max_retries_exceeded() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); let test_data = b"test data that will fail"; @@ -283,7 +285,7 @@ async fn test_upload_failure_max_retries_exceeded() { assert_eq!( store.get_attempt_count(), 3, - "Should have made exactly max_attempts" + "Should have made exactly max_retries" ); } @@ -318,10 +320,11 @@ async fn test_download_retry_transient_failures() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); // Download should succeed after retries (but retrieve doesn't use with_retry) @@ -358,10 +361,11 @@ async fn test_metadata_retry_transient_failures() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); // Metadata should succeed after retries (but metadata doesn't use with_retry) @@ -421,10 +425,11 @@ async fn test_retry_backoff_timing() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(50), + max_retries: 3, + base_delay: Duration::from_millis(50), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); let start = Instant::now(); @@ -449,10 +454,11 @@ async fn test_retry_config_validation() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 1, - initial_delay: Duration::from_millis(10), + max_retries: 1, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); let test_data = b"test data"; @@ -538,10 +544,11 @@ async fn test_retry_max_delay_capping() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 4, - initial_delay: Duration::from_millis(100), + max_retries: 4, + base_delay: Duration::from_millis(100), max_delay: Duration::from_millis(150), // Cap at 150ms backoff_multiplier: 10.0, // Very aggressive multiplier + ..Default::default() }); let start = std::time::Instant::now(); @@ -566,10 +573,11 @@ async fn test_upload_auth_error_no_retry() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }); let test_data = b"test data"; @@ -661,10 +669,11 @@ async fn test_concurrent_uploads_with_retry() { "test-bucket".to_string(), ) .with_retry_config(RetryConfig { - max_attempts: 3, - initial_delay: Duration::from_millis(10), + max_retries: 3, + base_delay: Duration::from_millis(10), max_delay: Duration::from_secs(1), backoff_multiplier: 2.0, + ..Default::default() }), );