refactor: consolidate RetryConfig to common::resilience
The storage crate had its own RetryConfig struct (max_attempts, initial_delay, max_delay, backoff_multiplier) duplicating common::resilience::retry::RetryConfig. Added backoff_multiplier field to common's RetryConfig and updated storage to re-export and use common's version with its field names (max_retries, base_delay). Updated all storage tests accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -134,15 +134,15 @@ impl ObjectStoreBackend {
|
||||
F: Fn() -> Fut,
|
||||
Fut: std::future::Future<Output = StorageResult<T>>,
|
||||
{
|
||||
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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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()
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user