diff --git a/crates/config/src/constants/app.rs b/crates/config/src/constants/app.rs index 6dd3cb6a5..36ea95d5a 100644 --- a/crates/config/src/constants/app.rs +++ b/crates/config/src/constants/app.rs @@ -353,6 +353,11 @@ pub const DEFAULT_OBS_TRACES_EXPORT_ENABLED: bool = true; /// Environment variable: RUSTFS_OBS_METRICS_EXPORT_ENABLED pub const DEFAULT_OBS_METRICS_EXPORT_ENABLED: bool = true; +/// Default detailed PUT stage metrics enabled +/// Default value: false +/// Environment variable: RUSTFS_OBS_PUT_STAGE_METRICS_ENABLED +pub const DEFAULT_OBS_PUT_STAGE_METRICS_ENABLED: bool = false; + /// Default logs export enabled /// It is used to enable or disable exporting logs /// Default value: true diff --git a/crates/config/src/observability/mod.rs b/crates/config/src/observability/mod.rs index 9dd2ba654..b23e5f2c6 100644 --- a/crates/config/src/observability/mod.rs +++ b/crates/config/src/observability/mod.rs @@ -44,6 +44,10 @@ pub const ENV_OBS_METRICS_EXPORT_ENABLED: &str = "RUSTFS_OBS_METRICS_EXPORT_ENAB pub const ENV_OBS_LOGS_EXPORT_ENABLED: &str = "RUSTFS_OBS_LOGS_EXPORT_ENABLED"; pub const ENV_OBS_PROFILING_EXPORT_ENABLED: &str = "RUSTFS_OBS_PROFILING_EXPORT_ENABLED"; +/// Enables detailed per-stage PUT metrics. Disabled by default because each +/// PUT records multiple timers and histograms when attribution is active. +pub const ENV_OBS_PUT_STAGE_METRICS_ENABLED: &str = "RUSTFS_OBS_PUT_STAGE_METRICS_ENABLED"; + pub const ENV_OBS_LOGGER_LEVEL: &str = "RUSTFS_OBS_LOGGER_LEVEL"; pub const ENV_OBS_LOG_STDOUT_ENABLED: &str = "RUSTFS_OBS_LOG_STDOUT_ENABLED"; pub const ENV_OBS_LOG_DIRECTORY: &str = "RUSTFS_OBS_LOG_DIRECTORY"; @@ -141,6 +145,7 @@ mod tests { assert_eq!(ENV_OBS_METRICS_EXPORT_ENABLED, "RUSTFS_OBS_METRICS_EXPORT_ENABLED"); assert_eq!(ENV_OBS_LOGS_EXPORT_ENABLED, "RUSTFS_OBS_LOGS_EXPORT_ENABLED"); assert_eq!(ENV_OBS_PROFILING_EXPORT_ENABLED, "RUSTFS_OBS_PROFILING_EXPORT_ENABLED"); + assert_eq!(ENV_OBS_PUT_STAGE_METRICS_ENABLED, "RUSTFS_OBS_PUT_STAGE_METRICS_ENABLED"); // Test log cleanup related env keys assert_eq!(ENV_OBS_LOG_MAX_TOTAL_SIZE_BYTES, "RUSTFS_OBS_LOG_MAX_TOTAL_SIZE_BYTES"); assert_eq!(ENV_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES, "RUSTFS_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES"); diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index d8b294dae..ffc175659 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -405,6 +405,8 @@ pub mod metrics { } pub mod notification { + #[cfg(any(test, feature = "test-util"))] + pub use crate::services::notification_sys::rotate_cross_pool_fence_fleet_proof_for_test; pub use crate::services::notification_sys::{ CrossPoolFenceFleetProofToken, NotificationPeerErr, NotificationSys, acquire_cross_pool_fence_fleet_proof, cross_pool_fence_fleet_proof_matches, get_global_notification_sys, new_global_notification_sys, @@ -468,7 +470,7 @@ pub mod set_disk { #[cfg(feature = "test-util")] pub mod test_util { pub use crate::bucket::quota::reservation::fail_next_quota_ledger_save_for_test; - pub use crate::set_disk::{PutObjectCommitBarrier, PutObjectCommitPause}; + pub use crate::set_disk::{MultipartCommitBarrier, MultipartCommitPause, PutObjectCommitBarrier, PutObjectCommitPause}; } } diff --git a/crates/ecstore/src/bucket/quota/checker.rs b/crates/ecstore/src/bucket/quota/checker.rs index d30e91935..ebb1b4dbb 100644 --- a/crates/ecstore/src/bucket/quota/checker.rs +++ b/crates/ecstore/src/bucket/quota/checker.rs @@ -52,6 +52,7 @@ impl QuotaChecker { ) -> Result { let start_time = Instant::now(); let quota_config = self.get_quota_config(bucket).await?; + let uses_durable_reservations = quota_config.uses_durable_reservations(); // If no quota limit is set, allow operation let quota_limit = match quota_config.quota { @@ -67,6 +68,7 @@ impl QuotaChecker { quota_limit: None, operation_size, remaining: None, + uses_durable_reservations, }); } Some(q) => q, @@ -74,14 +76,17 @@ impl QuotaChecker { let current_usage = self.get_real_time_usage(bucket).await?; + let admission_size = if uses_durable_reservations { 0 } else { operation_size }; let expected_usage = match operation { - QuotaOperation::PutObject | QuotaOperation::PostObject | QuotaOperation::CopyObject => current_usage + operation_size, + QuotaOperation::PutObject | QuotaOperation::PostObject | QuotaOperation::CopyObject => { + current_usage.saturating_add(admission_size) + } QuotaOperation::DeleteObject => current_usage.saturating_sub(operation_size), }; let allowed = match operation { QuotaOperation::PutObject | QuotaOperation::PostObject | QuotaOperation::CopyObject => { - quota_config.check_operation_allowed(current_usage, operation_size) + quota_config.check_operation_allowed(current_usage, admission_size) } QuotaOperation::DeleteObject => true, }; @@ -105,6 +110,7 @@ impl QuotaChecker { quota_limit: Some(quota_limit), operation_size, remaining, + uses_durable_reservations, }; let duration = start_time.elapsed(); @@ -375,6 +381,7 @@ mod tests { quota_limit: None, operation_size: 1024, remaining: None, + uses_durable_reservations: false, }; assert!(result.allowed); @@ -398,4 +405,13 @@ mod tests { let allowed = quota.check_operation_allowed(512, 1024); assert!(!allowed); } + + #[test] + fn legacy_quota_rejects_full_operation_while_v1_defers_net_growth() { + let legacy: BucketQuota = serde_json::from_str(r#"{"quota":5}"#).expect("legacy quota should parse"); + let durable = BucketQuota::new(Some(5)); + + assert!(!legacy.check_operation_allowed(4, 2)); + assert!(durable.uses_durable_reservations()); + } } diff --git a/crates/ecstore/src/bucket/quota/mod.rs b/crates/ecstore/src/bucket/quota/mod.rs index a4c40faf5..157c750e4 100644 --- a/crates/ecstore/src/bucket/quota/mod.rs +++ b/crates/ecstore/src/bucket/quota/mod.rs @@ -20,7 +20,7 @@ use rustfs_config::{ QUOTA_API_PATH, QUOTA_EXCEEDED_ERROR_CODE, QUOTA_INTERNAL_ERROR_CODE, QUOTA_INVALID_CONFIG_ERROR_CODE, QUOTA_NOT_FOUND_ERROR_CODE, }; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer, de::Error as _}; use thiserror::Error; use time::OffsetDateTime; @@ -90,7 +90,10 @@ impl<'de> Deserialize<'de> for BucketQuota { { let wire = BucketQuotaWire::deserialize(deserializer)?; let quota = if wire.reservation_protocol == Some(QUOTA_RESERVATION_PROTOCOL_V1) { - wire.reservation_quota + Some( + wire.reservation_quota + .ok_or_else(|| D::Error::custom("reservation_quota is required for reservation protocol v1"))?, + ) } else { wire.quota }; @@ -164,6 +167,7 @@ pub struct QuotaCheckResult { pub quota_limit: Option, pub operation_size: u64, pub remaining: Option, + pub uses_durable_reservations: bool, } #[derive(Debug)] @@ -327,6 +331,14 @@ mod tests { assert!(quota.has_unsupported_reservation_protocol()); } + #[test] + fn reservation_protocol_v1_requires_reservation_quota() { + let err = serde_json::from_str::(r#"{"quota":0,"quota_type":"Hard","reservation_protocol":1}"#) + .expect_err("v1 without its authoritative quota must fail closed"); + + assert!(err.to_string().contains("reservation_quota is required")); + } + /// unmarshal accepts format without quota_type #[test] fn unmarshal_format_without_quota_type() { diff --git a/crates/ecstore/src/bucket/quota/reservation.rs b/crates/ecstore/src/bucket/quota/reservation.rs index e1119d12c..15ec725cd 100644 --- a/crates/ecstore/src/bucket/quota/reservation.rs +++ b/crates/ecstore/src/bucket/quota/reservation.rs @@ -209,6 +209,7 @@ pub(crate) struct QuotaContext { quota_limit: Option, capability_proof: Option, snapshot_admission: Option, + legacy_data_movement: bool, metadata_guard: Option, pool_index: Option, set_index: Option, @@ -233,6 +234,12 @@ impl QuotaContext { } return Ok(QuotaReservation::unlimited(self.metadata_guard)); } + if self.legacy_data_movement { + if new_size > old_size { + return Err(StorageError::PartMissingOrCorrupt); + } + return Ok(QuotaReservation::unlimited(self.metadata_guard)); + } let store = self.store.ok_or(StorageError::PartMissingOrCorrupt)?; let bucket_incarnation = self.bucket_incarnation.ok_or(StorageError::PartMissingOrCorrupt)?; let quota_revision = self.quota_revision.ok_or(StorageError::PartMissingOrCorrupt)?; @@ -431,7 +438,8 @@ pub(crate) async fn begin( ctx: &crate::runtime::instance::InstanceContext, bucket: &str, object: &str, - _snapshot_admission: Option, + snapshot_admission: Option, + data_movement: bool, pool_index: usize, set_index: usize, ) -> Result { @@ -446,13 +454,14 @@ pub(crate) async fn begin( quota_limit: None, capability_proof: None, snapshot_admission: None, + legacy_data_movement: false, metadata_guard: None, pool_index: None, set_index: None, }); } #[cfg(test)] - if let Some(snapshot_admission) = _snapshot_admission { + if let Some(snapshot_admission) = snapshot_admission { return Ok(QuotaContext { store: None, bucket: bucket.to_string(), @@ -463,6 +472,7 @@ pub(crate) async fn begin( quota_limit: Some(snapshot_admission.quota_limit()), capability_proof: None, snapshot_admission: Some(snapshot_admission), + legacy_data_movement: false, metadata_guard: None, pool_index: Some(pool_index), set_index: Some(set_index), @@ -480,6 +490,7 @@ pub(crate) async fn begin( quota_limit: None, capability_proof: None, snapshot_admission: None, + legacy_data_movement: false, metadata_guard: None, pool_index: None, set_index: None, @@ -504,7 +515,8 @@ pub(crate) async fn begin( { return Err(StorageError::PartMissingOrCorrupt); } - let capability_proof = if quota.as_ref().is_some_and(|quota| quota.uses_durable_reservations()) { + let durable_quota = quota.as_ref().filter(|quota| quota.uses_durable_reservations()); + let capability_proof = if durable_quota.is_some() { Some( crate::services::notification_sys::acquire_cross_pool_fence_fleet_proof() .ok_or_else(|| quota_capability_error(bucket, &ledger_object(bucket)))?, @@ -512,10 +524,28 @@ pub(crate) async fn begin( } else { None }; - let quota_limit = quota - .filter(crate::bucket::quota::BucketQuota::uses_durable_reservations) - .and_then(|quota| quota.quota); - let store = if quota_limit.is_some() { + let durable_quota_limit = durable_quota.and_then(|quota| quota.quota); + let snapshot_admission = match quota.as_ref().filter(|quota| !quota.uses_durable_reservations()) { + Some(quota) => match (quota.quota, snapshot_admission) { + (Some(limit), Some(admission)) if admission.quota_limit() == limit => Some(admission), + (Some(_), None) if data_movement => None, + (Some(_), _) => return Err(StorageError::PartMissingOrCorrupt), + (None, _) => None, + }, + None => None, + }; + let legacy_data_movement = durable_quota_limit.is_none() + && quota.as_ref().and_then(|quota| quota.quota).is_some() + && snapshot_admission.is_none() + && data_movement; + let quota_limit = durable_quota_limit + .or_else(|| snapshot_admission.map(QuotaAdmission::quota_limit)) + .or_else(|| { + legacy_data_movement + .then(|| quota.as_ref().and_then(|quota| quota.quota)) + .flatten() + }); + let store = if durable_quota_limit.is_some() { Some(metadata_sys::object_store_in(ctx).await?) } else { None @@ -529,7 +559,8 @@ pub(crate) async fn begin( quota_revision: Some(quota_revision), quota_limit, capability_proof, - snapshot_admission: None, + snapshot_admission, + legacy_data_movement, metadata_guard: Some(metadata_guard), pool_index: Some(pool_index), set_index: Some(set_index), diff --git a/crates/ecstore/src/cluster/rpc/remote_disk.rs b/crates/ecstore/src/cluster/rpc/remote_disk.rs index 006ac9090..7a2c1a3a8 100644 --- a/crates/ecstore/src/cluster/rpc/remote_disk.rs +++ b/crates/ecstore/src/cluster/rpc/remote_disk.rs @@ -846,31 +846,49 @@ impl RemoteDisk { /// default to 1 (see [`internode_idempotent_read_retries`]). MUST NOT be used for write/lock /// RPCs — those must never auto-retry (quorum/idempotency safety). The `operation` closure is /// re-invoked per attempt, so it must be `Fn` (rebuild the request from borrowed inputs, do not - /// move captured state out). + /// move captured state out). Attempts and backoff share one total timeout budget. async fn execute_read_with_retry(&self, op: &'static str, operation: F, timeout_duration: Duration) -> Result where F: Fn() -> Fut, Fut: std::future::Future>, { + let deadline = (!timeout_duration.is_zero()).then(|| { + time::Instant::now() + .checked_add(timeout_duration) + .unwrap_or_else(|| time::sleep(timeout_duration).deadline()) + }); let max_retries = internode_idempotent_read_retries(); let mut attempt = 0usize; loop { - // Only the final attempt marks the disk faulty / evicts the channel. Earlier retries - // ignore the failure, so a transient error cannot flip the disk into a faulty - // short-circuit (which would defeat the retry) or over-count failures. + let attempt_timeout = deadline + .map(|deadline| deadline.saturating_duration_since(time::Instant::now())) + .unwrap_or(Duration::ZERO); + if deadline.is_some() && attempt_timeout.is_zero() { + self.record_timeout(op, timeout_duration); + return Err(DiskError::Timeout); + } + let health_action = if attempt >= max_retries { FailureHealthAction::MarkFailure } else { FailureHealthAction::IgnoreFailure }; match self - .execute_with_timeout_for_op_and_health_action(op, &operation, timeout_duration, health_action) + .execute_with_timeout_for_op_and_health_action(op, &operation, attempt_timeout, health_action) .await { Err(err) if attempt < max_retries && is_network_like_disk_error(&err) => { + if matches!(err, DiskError::Timeout) && deadline.is_some_and(|deadline| time::Instant::now() >= deadline) { + self.mark_faulty("read_operation_deadline"); + return Err(err); + } attempt += 1; let backoff = REMOTE_DISK_READ_RETRY_BASE_BACKOFF .saturating_mul(1u32 << u32::try_from(attempt - 1).unwrap_or(4).min(4)); + if deadline.is_some_and(|deadline| deadline.saturating_duration_since(time::Instant::now()) <= backoff) { + attempt = max_retries; + continue; + } debug!( endpoint = %self.endpoint, addr = %self.addr, @@ -878,7 +896,17 @@ impl RemoteDisk { attempt, "retrying idempotent read-only RPC after transient network error" ); - tokio::time::sleep(backoff).await; + if let Some(deadline) = deadline { + if time::timeout_at(deadline, time::sleep(backoff)).await.is_err() { + self.record_timeout(op, timeout_duration); + return Err(DiskError::Timeout); + } + } else { + time::sleep(backoff).await; + } + if self.health.is_faulty() { + return Err(DiskError::FaultyDisk); + } } other => return other, } @@ -957,32 +985,35 @@ impl RemoteDisk { operation_result } Err(_) => { - // Timeout occurred, mark disk as potentially faulty - counter!( - "rustfs_drive_op_timeout_total", - "endpoint" => self.endpoint.to_string(), - "op" => op.to_string() - ) - .increment(1); + self.record_timeout(op, timeout_duration); if failure_health_action == FailureHealthAction::MarkFailure { self.mark_faulty_and_evict("operation_timeout").await; } - warn!( - event = EVENT_REMOTE_DISK_RPC, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_REMOTE_DISK, - endpoint = %self.endpoint, - addr = %self.addr, - op, - timeout_ms = timeout_duration.as_millis(), - state = "timeout", - "Remote disk operation timed out" - ); Err(DiskError::Timeout) } } } + fn record_timeout(&self, op: &'static str, timeout_duration: Duration) { + counter!( + "rustfs_drive_op_timeout_total", + "endpoint" => self.endpoint.to_string(), + "op" => op.to_string() + ) + .increment(1); + warn!( + event = EVENT_REMOTE_DISK_RPC, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_REMOTE_DISK, + endpoint = %self.endpoint, + addr = %self.addr, + op, + timeout_ms = timeout_duration.as_millis(), + state = "timeout", + "Remote disk operation timed out" + ); + } + async fn handle_network_like_error( &self, op: &'static str, @@ -1016,7 +1047,7 @@ impl RemoteDisk { } } - async fn mark_faulty_and_evict(&self, reason: &'static str) { + fn mark_faulty(&self, reason: &'static str) -> bool { let previous_state = self.runtime_state(); let transitioned_to_offline = self.mark_suspect_or_offline(reason); let state = self.runtime_state(); @@ -1053,6 +1084,12 @@ impl RemoteDisk { "Remote disk marked suspect" ); } + } + state != previous_state + } + + async fn mark_faulty_and_evict(&self, reason: &'static str) { + if self.mark_faulty(reason) { counter!( "rustfs_drive_connection_evict_total", "endpoint" => self.endpoint.to_string(), @@ -2068,7 +2105,7 @@ impl DiskAPI for RemoteDisk { Ok(file_info) }, - get_max_timeout_duration(), + get_drive_metadata_timeout(), ) .await } @@ -5094,6 +5131,452 @@ mod tests { ); } + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_reset_during_backoff_preserves_recovery() { + let remote_disk = Arc::new(new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await); + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let first_attempt = Arc::new(tokio::sync::Notify::new()); + let started = time::Instant::now(); + + let task_disk = Arc::clone(&remote_disk); + let task_attempts = Arc::clone(&attempts); + let task_first_attempt = Arc::clone(&first_attempt); + let task = tokio::spawn(async move { + task_disk + .execute_read_with_retry( + "read_version", + move || { + let attempt = task_attempts.fetch_add(1, Ordering::SeqCst); + let first_attempt = Arc::clone(&task_first_attempt); + async move { + if attempt == 0 { + time::sleep(Duration::from_millis(20)).await; + first_attempt.notify_one(); + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionRefused, + "connection refused", + ))); + } + Ok(()) + } + }, + Duration::from_millis(100), + ) + .await + }); + + first_attempt.notified().await; + tokio::task::yield_now().await; + remote_disk.health.reset_for_store_init_retry(&remote_disk.endpoint); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + task.await + .expect("retry task should finish") + .expect("the retry should succeed after the health reset"); + + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(started.elapsed(), Duration::from_millis(70)); + assert_eq!( + remote_disk.health.waiting_count(), + 0, + "health reset must not underflow the waiting counter" + ); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Online); + assert!( + runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await, + "a recovered channel must survive the retry backoff" + ); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_still_retries_within_shared_deadline() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + + remote_disk + .execute_read_with_retry( + "read_version", + || { + let attempt = attempts.fetch_add(1, Ordering::SeqCst); + async move { + if attempt == 0 { + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionReset, + "connection reset", + ))); + } + Ok(()) + } + }, + Duration::from_millis(100), + ) + .await + .expect("a retry that fits the shared deadline should succeed"); + + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Online); + assert!(runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_uses_remaining_budget_for_final_attempt() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + let attempt = attempts.fetch_add(1, Ordering::SeqCst); + async move { + if attempt == 0 { + time::sleep(Duration::from_millis(20)).await; + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionRefused, + "connection refused", + ))); + } + std::future::pending::>().await + } + }, + Duration::from_millis(100), + ) + .await + .expect_err("the final retry should consume only the remaining total budget"); + + assert_eq!(err, DiskError::Timeout); + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(started.elapsed(), Duration::from_millis(100)); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + assert!(!runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_uses_final_attempt_at_exact_backoff_boundary() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + let attempt = attempts.fetch_add(1, Ordering::SeqCst); + async move { + if attempt == 0 { + time::sleep(Duration::from_millis(50)).await; + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionRefused, + "connection refused", + ))); + } + std::future::pending::>().await + } + }, + Duration::from_millis(100), + ) + .await + .expect_err("the exact backoff boundary should be reserved for a final attempt"); + + assert_eq!(err, DiskError::Timeout); + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(started.elapsed(), Duration::from_millis(100)); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_uses_final_attempt_below_backoff_budget() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + let attempt = attempts.fetch_add(1, Ordering::SeqCst); + async move { + if attempt == 0 { + time::sleep(Duration::from_millis(80)).await; + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionRefused, + "connection refused", + ))); + } + std::future::pending::>().await + } + }, + Duration::from_millis(100), + ) + .await + .expect_err("remaining budget below backoff should be reserved for a final attempt"); + + assert_eq!(err, DiskError::Timeout); + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(started.elapsed(), Duration::from_millis(100)); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_zero_timeout_disables_the_deadline() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + + remote_disk + .execute_read_with_retry( + "read_version", + || { + let attempt = attempts.fetch_add(1, Ordering::SeqCst); + async move { + if attempt == 0 { + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionReset, + "connection reset", + ))); + } + Ok(()) + } + }, + Duration::ZERO, + ) + .await + .expect("zero timeout should allow a retry without a deadline"); + + assert_eq!(attempts.load(Ordering::SeqCst), 2); + assert_eq!(started.elapsed(), REMOTE_DISK_READ_RETRY_BASE_BACKOFF); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_accepts_max_metadata_timeout() { + temp_env::async_with_vars([(rustfs_config::ENV_DRIVE_METADATA_TIMEOUT_SECS, Some(u64::MAX.to_string()))], async { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + + remote_disk + .execute_read_with_retry("read_version", || async { Ok::<(), Error>(()) }, get_drive_metadata_timeout()) + .await + .expect("the maximum configured metadata timeout must not panic"); + + remote_disk.cancel_token.cancel(); + }) + .await; + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_zero_retries_runs_once() { + temp_env::async_with_vars([(rustfs_config::ENV_INTERNODE_IDEMPOTENT_READ_RETRIES, Some("0"))], async { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + attempts.fetch_add(1, Ordering::SeqCst); + async { + Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionReset, + "connection reset", + ))) + } + }, + Duration::from_secs(1), + ) + .await + .expect_err("zero retries should return the first network error"); + + assert!(matches!(err, DiskError::Io(ref io_err) if io_err.kind() == std_io::ErrorKind::ConnectionReset)); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + assert_eq!(started.elapsed(), Duration::ZERO); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + assert!(!runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await); + remote_disk.cancel_token.cancel(); + }) + .await; + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_attempt_timeout_marks_health_without_evicting() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let recorder = crate::test_metrics::CapturingRecorder::default(); + let _recorder_guard = metrics::set_default_local_recorder(&recorder); + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + attempts.fetch_add(1, Ordering::SeqCst); + std::future::pending::>() + }, + Duration::from_millis(100), + ) + .await + .expect_err("an in-flight attempt that consumes the deadline should time out"); + + assert_eq!(err, DiskError::Timeout); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + assert!(runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await); + assert_eq!( + recorder.counter_value( + "rustfs_drive_op_timeout_total", + &[ + ("endpoint", remote_disk.endpoint.to_string().as_str()), + ("op", "read_version") + ] + ), + 1 + ); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_does_not_retry_business_errors() { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + attempts.fetch_add(1, Ordering::SeqCst); + async { Err::<(), Error>(DiskError::FileNotFound) } + }, + Duration::from_secs(1), + ) + .await + .expect_err("business errors should be returned directly"); + + assert_eq!(err, DiskError::FileNotFound); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + remote_disk.cancel_token.cancel(); + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_honors_configured_retry_count() { + temp_env::async_with_vars([(rustfs_config::ENV_INTERNODE_IDEMPOTENT_READ_RETRIES, Some("2"))], async { + let remote_disk = new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await; + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let started = time::Instant::now(); + let channel = TonicEndpoint::from_shared(remote_disk.addr.clone()) + .expect("remote disk address should parse") + .connect_lazy(); + runtime_sources::cache_test_node_channel(remote_disk.addr.clone(), channel).await; + + let err = remote_disk + .execute_read_with_retry( + "read_version", + || { + attempts.fetch_add(1, Ordering::SeqCst); + async { + Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionReset, + "connection reset", + ))) + } + }, + Duration::from_secs(1), + ) + .await + .expect_err("exhausted retries should return the last network error"); + + assert!(matches!(err, DiskError::Io(ref io_err) if io_err.kind() == std_io::ErrorKind::ConnectionReset)); + assert_eq!(attempts.load(Ordering::SeqCst), 3); + assert_eq!(started.elapsed(), Duration::from_millis(150)); + assert_eq!(remote_disk.runtime_state(), RuntimeDriveHealthState::Suspect); + assert!(!runtime_sources::test_node_channel_is_cached(&remote_disk.addr).await); + remote_disk.cancel_token.cancel(); + }) + .await; + } + + #[tokio::test(start_paused = true)] + #[serial(remote_disk_read_retry)] + async fn execute_read_with_retry_stops_when_disk_turns_offline_during_backoff() { + let remote_disk = Arc::new(new_remote_disk_with_transport(Arc::new(RecordingInternodeDataTransport::default())).await); + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let first_attempt = Arc::new(tokio::sync::Notify::new()); + let task_disk = Arc::clone(&remote_disk); + let task_attempts = Arc::clone(&attempts); + let task_first_attempt = Arc::clone(&first_attempt); + + let task = tokio::spawn(async move { + task_disk + .execute_read_with_retry( + "read_version", + move || { + let attempt = task_attempts.fetch_add(1, Ordering::SeqCst); + let first_attempt = Arc::clone(&task_first_attempt); + async move { + if attempt == 0 { + first_attempt.notify_one(); + return Err::<(), Error>(DiskError::Io(std_io::Error::new( + std_io::ErrorKind::ConnectionReset, + "connection reset", + ))); + } + Ok(()) + } + }, + Duration::from_secs(1), + ) + .await + }); + + first_attempt.notified().await; + tokio::task::yield_now().await; + remote_disk + .health + .force_runtime_state_for_test(RuntimeDriveHealthState::Offline); + time::advance(REMOTE_DISK_READ_RETRY_BASE_BACKOFF).await; + let err = task + .await + .expect("retry task should finish") + .expect_err("an offline disk must stop before the next attempt"); + + assert_eq!(err, DiskError::FaultyDisk); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + remote_disk.cancel_token.cancel(); + } + #[tokio::test] async fn test_execute_with_timeout_evicts_cached_connection() { let addr = "http://127.0.0.1:59991".to_string(); @@ -5603,6 +6086,40 @@ mod tests { accept_task.abort(); } + #[tokio::test] + async fn read_version_uses_the_metadata_timeout_on_a_stalled_peer() { + runtime_sources::ensure_test_rpc_secret(); + let Some((base_addr, accept_task)) = spawn_stalled_grpc_peer().await else { + return; + }; + let remote_disk = remote_disk_for_addr(&base_addr).await; + + temp_env::async_with_vars( + [ + (rustfs_config::ENV_DRIVE_METADATA_TIMEOUT_SECS, Some("1")), + (rustfs_config::ENV_DRIVE_MAX_TIMEOUT_DURATION, Some("10")), + ], + async { + let started = time::Instant::now(); + let err = tokio::time::timeout( + Duration::from_secs(5), + remote_disk.read_version("bucket", "bucket", "object", "", &ReadOptions::default()), + ) + .await + .expect("read_version must use the shorter metadata deadline") + .expect_err("a stalled peer must fail read_version"); + + assert!(matches!(err, DiskError::Timeout), "expected the metadata deadline to fire, got {err:?}"); + assert!(started.elapsed() >= Duration::from_millis(900)); + assert!(started.elapsed() < Duration::from_secs(2)); + }, + ) + .await; + + remote_disk.cancel_token.cancel(); + accept_task.abort(); + } + #[tokio::test] async fn delete_volume_bounds_the_wait_on_a_stalled_peer() { runtime_sources::ensure_test_rpc_secret(); diff --git a/crates/ecstore/src/disk/local.rs b/crates/ecstore/src/disk/local.rs index 8cdd45b89..d4bf38e01 100644 --- a/crates/ecstore/src/disk/local.rs +++ b/crates/ecstore/src/disk/local.rs @@ -9243,7 +9243,7 @@ impl DiskAPI for LocalDisk { if let Some(src_file_path_parent) = src_file_path.parent() { if src_volume != super::RUSTFS_META_MULTIPART_BUCKET { - let _ = remove_std(src_file_path_parent); + let _ = std::fs::remove_dir(src_file_path_parent); } else { let _ = self .delete_file(&dst_volume_dir, &src_file_path_parent.to_path_buf(), true, false) @@ -9568,7 +9568,7 @@ impl DiskAPI for LocalDisk { if let Some(ref cleanup) = cleanup_path { let _ = self.delete_file(&dst_volume_dir, cleanup, true, false).await; } else if let Some(parent) = src_file_path.parent() { - let _ = remove_std(parent); + let _ = std::fs::remove_dir(parent); } // Heal reuses a version's `data_dir` and lands the rebuilt shard on @@ -12565,6 +12565,10 @@ mod test { .join(RUSTFS_META_TMP_BUCKET) .join(tmp_object) .join(new_data_dir.to_string()); + let tmp_parent = tmp_data_dir + .parent() + .expect("tmp data dir should have a parent") + .to_path_buf(); fs::create_dir_all(&tmp_data_dir) .await .expect("new tmp data dir should be created"); @@ -12576,6 +12580,10 @@ mod test { disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, new_fi, bucket, object) .await .expect("rename_data should commit"); + assert!( + !tmp_parent.exists(), + "successful non-inline commit should remove the empty staging parent" + ); // The tmp xl.meta write point uses SyncMode::FileOnly: its parent dir // ({tmp}/{tmp_object}) must not be fsynced. @@ -12780,6 +12788,9 @@ mod test { let tmp_object = "tmp-new-inline"; ensure_test_volume(&disk, bucket).await; ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; + let tmp_parent = disk + .get_object_path(RUSTFS_META_TMP_BUCKET, tmp_object) + .expect("tmp parent should resolve"); let _mode = durability_mode_override::set(DurabilityMode::Strict); let version_id = Uuid::parse_str("99999999-9999-9999-9999-999999999999").expect("version id should parse"); @@ -12788,6 +12799,7 @@ mod test { disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, new_fi, bucket, object) .await .expect("inline rename_data should commit the new object"); + assert!(!tmp_parent.exists(), "successful inline commit should remove the empty staging parent"); let bucket_dir = disk.get_bucket_path(bucket).expect("bucket path should resolve"); let prefix_dir = disk.get_object_path(bucket, "prefix").expect("prefix path should resolve"); @@ -12811,6 +12823,34 @@ mod test { ); } + #[tokio::test] + async fn rename_data_inline_preserves_non_empty_staging_parent() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + let bucket = "inline-staging-sentinel-bucket"; + let object = "inline-object"; + let tmp_object = "inline-stage-with-sentinel"; + ensure_test_volume(&disk, bucket).await; + ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; + + let tmp_parent = disk + .get_object_path(RUSTFS_META_TMP_BUCKET, tmp_object) + .expect("tmp parent should resolve"); + fs::create_dir_all(&tmp_parent).await.expect("tmp parent should be created"); + let sentinel = tmp_parent.join("sentinel"); + fs::write(&sentinel, b"keep").await.expect("sentinel should be written"); + + let fi = test_file_info(object, Uuid::new_v4(), None, Some(Bytes::from_static(b"inline-payload"))); + disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, fi, bucket, object) + .await + .expect("non-empty staging cleanup must not negate the committed object"); + + assert_eq!(fs::read(&sentinel).await.expect("sentinel should remain"), b"keep"); + } + #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[allow(clippy::await_holding_lock)] @@ -12985,7 +13025,10 @@ mod test { .expect("non-inline rename_data should commit"); assert!(!replacement_dir.exists(), "the destination object directory must not be replaced"); - assert!(staging_parent.exists(), "the guarded staging parent must retain its identity"); + assert!( + !staging_parent.exists(), + "successful commit should remove the empty staging parent after releasing its guard" + ); assert!( !replacement_staging_parent.exists(), "the staging parent must not be replaced between data and metadata publication" diff --git a/crates/ecstore/src/erasure/coding/encode.rs b/crates/ecstore/src/erasure/coding/encode.rs index 4b03ad60e..39b792485 100644 --- a/crates/ecstore/src/erasure/coding/encode.rs +++ b/crates/ecstore/src/erasure/coding/encode.rs @@ -18,10 +18,12 @@ use crate::disk::error_reduce::{ }; use crate::erasure::coding::BitrotWriterWrapper; use crate::erasure::coding::Erasure; +use crate::erasure::coding::erasure::EncodedBlock; use crate::runtime::sources as runtime_sources; use bytes::{Bytes, BytesMut}; use futures::StreamExt; use futures::stream::FuturesUnordered; +use rustfs_utils::HashAlgorithm; use std::sync::Arc; use std::time::Instant; use std::vec; @@ -223,8 +225,8 @@ async fn send_queued( sender.send(InflightEntry::new(entry, bytes)).await } -fn queued_batch_bytes(batch: &[Vec]) -> usize { - batch.iter().map(|block| queued_block_bytes(block)).sum() +fn queued_batch_bytes(batch: &[EncodedBlock]) -> usize { + batch.iter().map(EncodedBlock::queued_bytes).sum() } fn dominant_error_summary_label(summary: &WriteQuorumFailureSummary) -> &'static str { @@ -336,7 +338,7 @@ impl<'a> MultiWriter<'a> { } } - async fn write_shard(writer_opt: &mut Option, err: &mut Option, shard: &Bytes) { + async fn write_shard(writer_opt: &mut Option, err: &mut Option, shard: &[u8]) { match writer_opt { Some(writer) => { match writer.write(shard).await { @@ -361,12 +363,20 @@ impl<'a> MultiWriter<'a> { } pub async fn write(&mut self, data: Vec) -> std::io::Result<()> { - assert_eq!(data.len(), self.writers.len()); + self.write_shards(data.iter().map(Bytes::as_ref)).await + } + + async fn write_block(&mut self, block: &EncodedBlock) -> std::io::Result<()> { + self.write_shards(block.shards()).await + } + + async fn write_shards<'b>(&mut self, shards: impl ExactSizeIterator) -> std::io::Result<()> { + assert_eq!(shards.len(), self.writers.len()); let budget = self.next_progress_budget(); { let mut futures = FuturesUnordered::new(); - for ((writer_opt, err), shard) in self.writers.iter_mut().zip(self.errs.iter_mut()).zip(data.iter()) { + for ((writer_opt, err), shard) in self.writers.iter_mut().zip(self.errs.iter_mut()).zip(shards) { if err.is_some() { continue; // Skip if we already have an error for this writer } @@ -490,10 +500,10 @@ impl<'a> MultiWriter<'a> { } impl Erasure { - async fn encode_block(self: Arc, encode_buf: Vec, len: usize) -> std::io::Result<(Vec, Vec)> { + async fn encode_block(self: Arc, encode_buf: Vec, len: usize) -> std::io::Result<(EncodedBlock, Vec)> { let encode_stage_start = stage_timer_if_enabled(); let encode_once = move || { - let res = self.encode_data(&encode_buf[..len]); + let res = self.encode_data_block(&encode_buf[..len]); (res, encode_buf) }; @@ -518,9 +528,9 @@ impl Erasure { Ok((res?, returned_buf)) } - async fn encode_block_bytes_mut(self: Arc, encode_buf: BytesMut, len: usize) -> std::io::Result> { + async fn encode_block_bytes_mut(self: Arc, encode_buf: BytesMut, len: usize) -> std::io::Result { let encode_stage_start = stage_timer_if_enabled(); - let encode_once = move || self.encode_data_bytes_mut(encode_buf, len); + let encode_once = move || self.encode_data_bytes_mut_block(encode_buf, len); let res = match tokio::runtime::Handle::current().runtime_flavor() { // Same rationale as encode_block: inline the short EC burst on the @@ -583,6 +593,39 @@ impl Erasure { Ok((reader, total)) } + /// Encode a small inline object directly into its per-disk bitrot payloads. + /// The returned bytes are the same `[hash][shard]` representation produced + /// by `BitrotWriter`, ready to be embedded in each disk's staged `xl.meta`. + #[hotpath::measure(impl_type = "Erasure")] + pub(crate) async fn encode_inline_shards_with_size_hint( + self: Arc, + mut reader: R, + size_hint: usize, + ) -> std::io::Result<(R, usize, Vec)> + where + R: AsyncRead + Send + Sync + Unpin, + { + use tokio::io::AsyncReadExt; + + let mut buf = Vec::with_capacity(small_ingest_capacity(&self, size_hint)); + let total = reader.read_to_end(&mut buf).await?; + if total == 0 { + return Ok((reader, 0, Vec::new())); + } + + let shards = self.encode_data_owned(buf)?; + let mut inline_shards = Vec::with_capacity(shards.len()); + for shard in shards { + let hash = HashAlgorithm::HighwayHash256S.hash_encode(&shard); + let mut encoded = BytesMut::with_capacity(hash.as_ref().len() + shard.len()); + encoded.extend_from_slice(hash.as_ref()); + encoded.extend_from_slice(&shard); + inline_shards.push(encoded.freeze()); + } + + Ok((reader, total, inline_shards)) + } + #[hotpath::measure(impl_type = "Erasure")] pub async fn encode( self: Arc, @@ -624,7 +667,7 @@ impl Erasure { let expanded_block_bytes = self.shard_size().saturating_mul(self.total_shard_count()); let max_inflight_bytes = erasure_encode_max_inflight_bytes(); let inflight_blocks = encode_channel_capacity(expanded_block_bytes, max_inflight_bytes); - let (tx, mut rx) = mpsc::channel::>>(inflight_blocks); + let (tx, mut rx) = mpsc::channel::>(inflight_blocks); let mut task = AbortOnDropTask::new(tokio::spawn(async move { let block_size = self.block_size; @@ -646,7 +689,7 @@ impl Erasure { let encode_buf = buf; let res = self.clone().encode_block_bytes_mut(encode_buf, n).await?; buf = BytesMut::with_capacity(ingest_capacity); - let queued_bytes = queued_block_bytes(&res); + let queued_bytes = res.queued_bytes(); let _producer_stage = rustfs_io_metrics::track_ec_encode_producer_bytes(queued_bytes); let send_wait_stage_start = stage_timer_if_enabled(); if let Err(err) = send_queued(&tx, res, queued_bytes).await { @@ -676,7 +719,7 @@ impl Erasure { let encode_buf = std::mem::take(&mut buf); let (res, returned_buf) = self.clone().encode_block(encode_buf, n).await?; buf = returned_buf; - let queued_bytes = queued_block_bytes(&res); + let queued_bytes = res.queued_bytes(); let _producer_stage = rustfs_io_metrics::track_ec_encode_producer_bytes(queued_bytes); let send_wait_stage_start = stage_timer_if_enabled(); if let Err(err) = send_queued(&tx, res, queued_bytes).await { @@ -720,9 +763,9 @@ impl Erasure { if block.is_empty() { break; } - let _writer_stage = rustfs_io_metrics::track_ec_encode_writer_bytes(queued_block_bytes(&block)); + let _writer_stage = rustfs_io_metrics::track_ec_encode_writer_bytes(block.queued_bytes()); let write_stage_start = stage_timer_if_enabled(); - if let Err(err) = writers.write(block).await { + if let Err(err) = writers.write_block(&block).await { write_err = Some(err); break; } @@ -769,7 +812,7 @@ impl Erasure { let inflight_blocks = encode_channel_capacity(expanded_block_bytes, max_inflight_bytes); let batch_blocks = encode_batch_block_count().min(inflight_blocks); let channel_capacity = inflight_blocks.div_ceil(batch_blocks).max(1); - let (tx, mut rx) = mpsc::channel::>>>(channel_capacity); + let (tx, mut rx) = mpsc::channel::>>(channel_capacity); let mut task = AbortOnDropTask::new(tokio::spawn(async move { let block_size = self.block_size; @@ -786,7 +829,7 @@ impl Erasure { let encode_buf = std::mem::take(&mut buf); let (res, returned_buf) = self.clone().encode_block(encode_buf, n).await?; buf = returned_buf; - let queued_bytes = queued_block_bytes(&res); + let queued_bytes = res.queued_bytes(); pending_batch_bytes = pending_batch_bytes.saturating_add(queued_bytes); pending_batch.push(res); drop(pending_batch_stage.take()); @@ -845,7 +888,7 @@ impl Erasure { let _writer_stage = rustfs_io_metrics::track_ec_encode_writer_bytes(queued_batch_bytes(&batch)); let write_stage_start = stage_timer_if_enabled(); for block in batch { - if let Err(err) = writers.write(block).await { + if let Err(err) = writers.write_block(&block).await { write_err = Some(err); break; } @@ -1895,7 +1938,11 @@ mod tests { let baseline = rustfs_io_metrics::current_ec_encode_inflight_bytes(); let (tx, rx) = mpsc::channel(2); let mut rx = rx; - let batch = vec![vec![Bytes::from_static(b"queued")], vec![Bytes::from_static(b"batch")]]; + let erasure = Erasure::new(1, 0, 16); + let batch = vec![ + erasure.encode_data_block(b"queued").expect("first block should encode"), + erasure.encode_data_block(b"batch").expect("second block should encode"), + ]; let batch_bytes = queued_batch_bytes(&batch); send_queued(&tx, batch, batch_bytes).await.expect("batch should be queued"); @@ -2236,11 +2283,11 @@ mod tests { .expect("bytesmut encode should succeed on current-thread runtime"); let expected_shard_size = payload.len().div_ceil(erasure.data_shards); - assert_eq!(shards.len(), erasure.total_shard_count()); - assert!(shards.iter().all(|shard| shard.len() == expected_shard_size)); + assert_eq!(shards.shards().len(), erasure.total_shard_count()); + assert!(shards.shards().all(|shard| shard.len() == expected_shard_size)); let mut restored = Vec::new(); - for shard in shards.iter().take(erasure.data_shards) { + for shard in shards.shards().take(erasure.data_shards) { restored.extend_from_slice(shard); } restored.truncate(payload.len()); @@ -2343,6 +2390,35 @@ mod tests { assert!(committed.lock().unwrap().is_empty()); } + #[tokio::test] + async fn encode_inline_shards_matches_writer_bitrot_layout() { + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const BLOCK_SIZE: usize = 64; + let payload = b"inline commit payload".to_vec(); + let checksum_algo = HashAlgorithm::HighwayHash256S; + let erasure = Arc::new(Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE)); + let reader = tokio::io::BufReader::new(Cursor::new(payload.clone())); + + let (_reader, total, inline_shards) = erasure + .clone() + .encode_inline_shards_with_size_hint(reader, payload.len()) + .await + .expect("inline shards should encode"); + let raw_shards = erasure + .encode_data_owned(payload.clone()) + .expect("reference shards should encode"); + + assert_eq!(total, payload.len()); + assert_eq!(inline_shards.len(), DATA_SHARDS + PARITY_SHARDS); + for (inline, raw) in inline_shards.iter().zip(raw_shards) { + let mut writer = BitrotWriterWrapper::new(CustomWriter::new_inline_buffer(), raw.len(), checksum_algo.clone()); + writer.write(&raw).await.expect("reference writer should accept shard"); + writer.shutdown().await.expect("reference writer should shutdown"); + assert_eq!(inline.as_ref(), writer.into_inline_data().expect("reference writer should retain bytes")); + } + } + /// encode_inline_small: small payload is encoded into the correct number of shards /// and each writer receives data after shutdown. #[tokio::test] @@ -2506,7 +2582,7 @@ mod tests { assert_eq!(&next[..], &data[16..]); } - async fn committed_shards_for_ingest_mode(use_bytesmut_ingest: bool, uses_legacy: bool, payload: &[u8]) -> Vec> { + async fn committed_shards_for_pipeline(pipeline: EncodePipeline, uses_legacy: bool, payload: &[u8]) -> Vec> { const DATA_SHARDS: usize = 2; const PARITY_SHARDS: usize = 2; const TOTAL_SHARDS: usize = DATA_SHARDS + PARITY_SHARDS; @@ -2520,10 +2596,16 @@ mod tests { let erasure = Arc::new(Erasure::new_with_options(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE, uses_legacy)); let reader = tokio::io::BufReader::new(Cursor::new(payload.to_vec())); - let (_reader, total) = erasure - .encode_with_ingest_mode(reader, &mut writers, DATA_SHARDS, use_bytesmut_ingest) - .await - .expect("encode should succeed"); + let (_reader, total) = match pipeline { + EncodePipeline::Vec => { + erasure + .encode_with_ingest_mode(reader, &mut writers, DATA_SHARDS, false) + .await + } + EncodePipeline::BytesMut => erasure.encode_with_ingest_mode(reader, &mut writers, DATA_SHARDS, true).await, + EncodePipeline::Batched => erasure.encode_batched(reader, &mut writers, DATA_SHARDS).await, + } + .expect("encode should succeed"); assert_eq!(total, payload.len()); committed @@ -2532,31 +2614,64 @@ mod tests { .collect() } - /// HP-10 (rustfs/backlog#931) merge gate: the BytesMut ingest path must produce - /// byte-for-byte identical shard streams to the default Vec ingest path, for both - /// legacy-aware shard-size formulas, across empty, sub-block, exactly-full-block, - /// and multi-block-with-partial-tail payloads. + async fn expected_committed_shards(uses_legacy: bool, payload: &[u8]) -> Vec> { + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const TOTAL_SHARDS: usize = DATA_SHARDS + PARITY_SHARDS; + const BLOCK_SIZE: usize = 64; + + let committed: Vec>>> = (0..TOTAL_SHARDS).map(|_| Arc::new(Mutex::new(Vec::new()))).collect(); + let mut writers: Vec = committed + .iter() + .map(|c| bitrot_writer(DeferredCommitWriter::new(c.clone()), BLOCK_SIZE / DATA_SHARDS)) + .collect(); + let erasure = Erasure::new_with_options(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE, uses_legacy); + + for block in payload.chunks(BLOCK_SIZE) { + let shards = erasure.encode_data(block).expect("reference block should encode"); + for (writer, shard) in writers.iter_mut().zip(shards) { + let written = writer.write(&shard).await.expect("reference shard should write"); + assert_eq!(written, shard.len()); + } + } + for writer in &mut writers { + writer.shutdown().await.expect("reference writer should commit"); + } + + committed + .iter() + .map(|c| c.lock().expect("committed buffer should be lockable").clone()) + .collect() + } + + /// The streaming and batched paths must produce the same bitrot-wrapped shard + /// bytes as the public block encoder for both shard-size formulas and all block + /// boundary shapes. #[tokio::test] async fn bytesmut_ingest_matches_vec_ingest_byte_for_byte() { const BLOCK_SIZE: usize = 64; let payloads: Vec> = vec![ Vec::new(), - b"tiny".to_vec(), + vec![1], + vec![2; BLOCK_SIZE - 1], (0..BLOCK_SIZE as u32).map(|i| i as u8).collect(), // exactly one full block - vec![3u8; BLOCK_SIZE * 4], // whole number of blocks + vec![4; BLOCK_SIZE + 1], + vec![3u8; BLOCK_SIZE * 4], // whole number of blocks (0..(BLOCK_SIZE * 3 + 7) as u32).map(|i| (i % 251) as u8).collect(), // partial tail ]; for uses_legacy in [false, true] { for payload in &payloads { - let vec_path = committed_shards_for_ingest_mode(false, uses_legacy, payload).await; - let bytesmut_path = committed_shards_for_ingest_mode(true, uses_legacy, payload).await; - assert_eq!( - vec_path, - bytesmut_path, - "ingest paths must be byte-identical (legacy={uses_legacy}, payload_len={})", - payload.len() - ); + let expected = expected_committed_shards(uses_legacy, payload).await; + for pipeline in [EncodePipeline::Vec, EncodePipeline::BytesMut, EncodePipeline::Batched] { + let actual = committed_shards_for_pipeline(pipeline, uses_legacy, payload).await; + assert_eq!( + actual, + expected, + "streaming shards must match the public block encoder (legacy={uses_legacy}, payload_len={})", + payload.len() + ); + } } } } diff --git a/crates/ecstore/src/erasure/coding/erasure.rs b/crates/ecstore/src/erasure/coding/erasure.rs index 8b4e213b2..23cac93c7 100644 --- a/crates/ecstore/src/erasure/coding/erasure.rs +++ b/crates/ecstore/src/erasure/coding/erasure.rs @@ -29,6 +29,46 @@ use tokio::io::AsyncRead; use tracing::warn; use uuid::Uuid; +pub(crate) struct EncodedBlock { + data: Bytes, + shard_size: usize, +} + +impl EncodedBlock { + fn empty() -> Self { + Self { + data: Bytes::new(), + shard_size: 0, + } + } + + pub(crate) fn is_empty(&self) -> bool { + self.data.is_empty() + } + + pub(crate) fn queued_bytes(&self) -> usize { + self.data.len() + } + + pub(crate) fn shards(&self) -> impl ExactSizeIterator { + debug_assert!(self.shard_size > 0, "only non-empty encoded blocks reach shard writers"); + debug_assert_eq!(self.data.len() % self.shard_size, 0); + self.data.chunks_exact(self.shard_size) + } + + fn into_shards(mut self, shard_count: usize) -> Vec { + if self.shard_size == 0 { + return vec![Bytes::new(); shard_count]; + } + + let mut shards = Vec::with_capacity(shard_count); + for _ in 0..shard_count { + shards.push(self.data.split_to(self.shard_size)); + } + shards + } +} + const MODERN_MAX_TOTAL_SHARDS: usize = ::ORDER; const MODERN_REED_SOLOMON_CACHE_MAX_ENTRIES: usize = 64; @@ -675,6 +715,17 @@ impl Erasure { #[tracing::instrument(level = "debug", skip_all, fields(data_len=data.len()))] #[hotpath::measure(impl_type = "Erasure")] pub fn encode_data(&self, data: &[u8]) -> io::Result> { + self.encode_data_block_inner(data) + .map(|block| block.into_shards(self.total_shard_count())) + } + + #[tracing::instrument(level = "debug", skip_all, fields(data_len=data.len()))] + #[hotpath::measure(label = "Erasure::encode_data", impl_type = "Erasure")] + pub(crate) fn encode_data_block(&self, data: &[u8]) -> io::Result { + self.encode_data_block_inner(data) + } + + fn encode_data_block_inner(&self, data: &[u8]) -> io::Result { let shard_size_fn = if self.uses_legacy { calc_shard_size_legacy } else { @@ -682,7 +733,7 @@ impl Erasure { }; let per_shard_size = shard_size_fn(data.len(), self.data_shards); if per_shard_size == 0 { - return Ok(vec![Bytes::new(); self.total_shard_count()]); + return Ok(EncodedBlock::empty()); } let need_total_size = per_shard_size * self.total_shard_count(); @@ -708,15 +759,10 @@ impl Erasure { } } - // Zero-copy split, all shards reference data_buffer - let mut data_buffer = data_buffer.freeze(); - let mut shards = Vec::with_capacity(self.total_shard_count()); - for _ in 0..self.total_shard_count() { - let shard = data_buffer.split_to(per_shard_size); - shards.push(shard); - } - - Ok(shards) + Ok(EncodedBlock { + data: data_buffer.freeze(), + shard_size: per_shard_size, + }) } /// Encode owned data, avoiding a copy when the caller already has a heap buffer. @@ -786,7 +832,17 @@ impl Erasure { /// `data_len <= block_size` — both shard-size formulas are monotone in /// `data_len` — so this function never reallocates the buffer. #[hotpath::measure(impl_type = "Erasure")] - pub fn encode_data_bytes_mut(&self, mut data_buffer: BytesMut, data_len: usize) -> io::Result> { + pub fn encode_data_bytes_mut(&self, data_buffer: BytesMut, data_len: usize) -> io::Result> { + self.encode_data_bytes_mut_block_inner(data_buffer, data_len) + .map(|block| block.into_shards(self.total_shard_count())) + } + + #[hotpath::measure(label = "Erasure::encode_data_bytes_mut", impl_type = "Erasure")] + pub(crate) fn encode_data_bytes_mut_block(&self, data_buffer: BytesMut, data_len: usize) -> io::Result { + self.encode_data_bytes_mut_block_inner(data_buffer, data_len) + } + + fn encode_data_bytes_mut_block_inner(&self, mut data_buffer: BytesMut, data_len: usize) -> io::Result { let shard_size_fn = if self.uses_legacy { calc_shard_size_legacy } else { @@ -794,7 +850,7 @@ impl Erasure { }; let per_shard_size = shard_size_fn(data_len, self.data_shards); if per_shard_size == 0 { - return Ok(vec![Bytes::new(); self.total_shard_count()]); + return Ok(EncodedBlock::empty()); } let need_total_size = per_shard_size * self.total_shard_count(); @@ -821,14 +877,10 @@ impl Erasure { } } - let mut data_buffer = data_buffer.freeze(); - let mut shards = Vec::with_capacity(self.total_shard_count()); - for _ in 0..self.total_shard_count() { - let shard = data_buffer.split_to(per_shard_size); - shards.push(shard); - } - - Ok(shards) + Ok(EncodedBlock { + data: data_buffer.freeze(), + shard_size: per_shard_size, + }) } /// Decode and reconstruct missing data shards in-place. @@ -1547,6 +1599,37 @@ mod tests { } } + #[test] + fn streaming_encoded_block_uses_one_contiguous_backing_buffer() { + let erasure = Erasure::new(8, 8, 64); + + for data_len in [1, 63, 64] { + let data = (0..data_len).map(|i| i as u8).collect::>(); + let expected = erasure.encode_data(&data).expect("public encode should succeed"); + let borrowed = erasure + .encode_data_block(&data) + .expect("borrowed streaming encode should succeed"); + let owned = erasure + .encode_data_bytes_mut_block(BytesMut::from(&data[..]), data.len()) + .expect("BytesMut streaming encode should succeed"); + + assert!(borrowed.shards().eq(expected.iter().map(Bytes::as_ref))); + assert!(owned.shards().eq(expected.iter().map(Bytes::as_ref))); + assert_eq!(borrowed.shards().len(), 16); + assert_eq!(borrowed.queued_bytes(), owned.queued_bytes()); + + let first = borrowed.shards().next().expect("encoded block should have shards").as_ptr(); + for (index, shard) in borrowed.shards().enumerate() { + assert_eq!(shard.as_ptr(), first.wrapping_add(index * shard.len())); + } + } + assert_eq!( + std::mem::size_of::(), + std::mem::size_of::() + std::mem::size_of::(), + "queue entries must contain one backing buffer handle, not per-shard handles" + ); + } + /// HP-10 capacity invariant: both shard-size formulas are monotone in `data_len`, /// so pre-reserving `shard_size(block_size) * total_shard_count` covers the /// `need_total_size` of every block-or-smaller payload and the ingest buffer diff --git a/crates/ecstore/src/services/notification_sys.rs b/crates/ecstore/src/services/notification_sys.rs index 272acc88f..8716c6ea0 100644 --- a/crates/ecstore/src/services/notification_sys.rs +++ b/crates/ecstore/src/services/notification_sys.rs @@ -210,6 +210,22 @@ pub fn cross_pool_fence_fleet_proof_matches(proof: &CrossPoolFenceFleetProofToke fleet_capability_proof_matches(cross_pool_fence_fleet_proof_slot(), &proof.0) } +#[cfg(any(test, feature = "test-util"))] +pub fn rotate_cross_pool_fence_fleet_proof_for_test() -> bool { + let mut state = cross_pool_fence_fleet_proof_slot() + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let Some(current) = state.proof.as_ref() else { + return false; + }; + state.proof = Some(FleetCapabilityProof { + topology_fingerprint: current.topology_fingerprint.clone(), + peer_epochs: Arc::new(current.peer_epochs.as_ref().clone()), + expires_at: current.expires_at, + }); + true +} + fn fleet_capability_proof_matches( slot: &std::sync::RwLock, proof: &FleetCapabilityProofToken, diff --git a/crates/ecstore/src/set_disk/metadata.rs b/crates/ecstore/src/set_disk/metadata.rs index a4c4dfa66..b07ed428c 100644 --- a/crates/ecstore/src/set_disk/metadata.rs +++ b/crates/ecstore/src/set_disk/metadata.rs @@ -1079,6 +1079,25 @@ impl SetDisks { shuffled_disks } + pub(super) fn shuffle_disks_owned(mut disks: Vec>, distribution: &[usize]) -> Vec> { + if distribution.is_empty() { + return disks; + } + + let mut shuffled_disks = vec![None; disks.len()]; + for (index, disk) in disks.iter_mut().enumerate() { + let Some(slot) = distribution + .get(index) + .and_then(|block_index| block_index.checked_sub(1)) + .filter(|slot| *slot < shuffled_disks.len()) + else { + continue; + }; + shuffled_disks[slot] = disk.take(); + } + shuffled_disks + } + pub(super) fn shuffle_check_parts(parts_errs: &[usize], distribution: &[usize]) -> Vec { if distribution.is_empty() { return parts_errs.to_vec(); @@ -1390,6 +1409,23 @@ mod tests { assert_eq!(owned_slots, expected_slots, "fallback disk slots must match the borrowing variant"); } + #[tokio::test] + async fn owned_shuffle_preserves_fresh_put_metadata() { + let tempdir = tempfile::tempdir().expect("tempdir should be created"); + let fi = FileInfo::new("bucket/object", 2, 1); + let parts = vec![fi.clone(); fi.erasure.distribution.len()]; + let disks = shuffle_test_disks(&tempdir, parts.len()).await; + + let (owned_disks, owned_parts) = SetDisks::shuffle_disks_and_parts_metadata_by_index_owned(disks, parts, &fi); + + assert!(owned_disks.iter().all(Option::is_some), "fresh PUT must retain every online disk"); + assert_eq!( + owned_parts, + vec![fi; owned_disks.len()], + "fresh PUT metadata with pending shard indexes must survive init fallback" + ); + } + // backlog#949: corrupt/adversarial distribution values (0 or > N) must not // trigger a `usize` underflow / out-of-bounds panic in the shuffle helpers. #[test] @@ -1419,6 +1455,22 @@ mod tests { assert_eq!(result.len(), disks.len(), "output length must be preserved"); } + #[tokio::test] + async fn owned_disk_shuffle_matches_borrowing_variant() { + let tempdir = tempfile::tempdir().expect("tempdir should be created"); + let mut disks = shuffle_test_disks(&tempdir, 4).await; + disks[1] = None; + disks[3] = None; + let distribution = [3, 1, 4, 2]; + + let expected = SetDisks::shuffle_disks(&disks, &distribution); + let actual = SetDisks::shuffle_disks_owned(disks, &distribution); + + let expected_slots = expected.iter().map(Option::is_some).collect::>(); + let actual_slots = actual.iter().map(Option::is_some).collect::>(); + assert_eq!(actual_slots, expected_slots, "owned shuffle must preserve disk placement"); + } + #[tokio::test] async fn shuffle_disks_and_parts_metadata_survives_corrupt_distribution() { let tempdir = tempfile::tempdir().expect("tempdir should be created"); diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index 9a1a29ee6..c95fba453 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -718,8 +718,8 @@ pub(crate) use core::io_primitives::disk_call_counters; mod ctx; mod metadata; mod ops; -#[cfg(test)] -pub(crate) use ops::multipart::{MultipartCommitBarrier, MultipartCommitPause}; +#[cfg(any(test, feature = "test-util"))] +pub use ops::multipart::{MultipartCommitBarrier, MultipartCommitPause}; #[cfg(feature = "test-util")] pub(crate) use ops::object::TransitionCleanupStoreBarrier as SetDiskTransitionCleanupStoreBarrier; pub(crate) use ops::object::body_cache_plaintext_len; diff --git a/crates/ecstore/src/set_disk/ops/multipart.rs b/crates/ecstore/src/set_disk/ops/multipart.rs index 4f9e255ba..d4a5654bd 100644 --- a/crates/ecstore/src/set_disk/ops/multipart.rs +++ b/crates/ecstore/src/set_disk/ops/multipart.rs @@ -27,24 +27,25 @@ use crate::crash_inject::{self, CrashPoint}; use crate::multipart_listing::paginate_multipart_listing; use futures::{StreamExt, stream}; use std::future::Future; -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; use tokio::task::JoinSet; const MULTIPART_LIST_IO_CONCURRENCY: usize = 16; -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] #[derive(Clone, Copy, PartialEq, Eq)] -pub(crate) enum MultipartCommitPause { +pub enum MultipartCommitPause { PutPartBeforeLockAcquire, PutPartBeforeLockLost, PutPartAfterRename, BeforeLockLost, + BeforeQuotaRename, AfterRename, } -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] struct MultipartCommitBarrierState { bucket: String, object: String, @@ -55,27 +56,22 @@ struct MultipartCommitBarrierState { release: tokio::sync::Semaphore, } -#[cfg(test)] -pub(crate) struct MultipartCommitBarrier { +#[cfg(any(test, feature = "test-util"))] +pub struct MultipartCommitBarrier { state: Arc, } -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] static MULTIPART_COMMIT_BARRIER: std::sync::OnceLock>>> = std::sync::OnceLock::new(); -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] impl MultipartCommitBarrier { - pub(crate) fn install(bucket: &str, object: &str, pause: MultipartCommitPause) -> Self { + pub fn install(bucket: &str, object: &str, pause: MultipartCommitPause) -> Self { Self::install_for_arrivals(bucket, object, pause, 1) } - pub(crate) fn install_for_arrivals( - bucket: &str, - object: &str, - pause: MultipartCommitPause, - expected_arrivals: usize, - ) -> Self { + pub fn install_for_arrivals(bucket: &str, object: &str, pause: MultipartCommitPause, expected_arrivals: usize) -> Self { assert!(expected_arrivals > 0, "multipart commit barrier must wait for at least one arrival"); let state = Arc::new(MultipartCommitBarrierState { bucket: bucket.to_string(), @@ -96,7 +92,7 @@ impl MultipartCommitBarrier { Self { state } } - pub(crate) async fn wait_until_paused(&self) { + pub async fn wait_until_paused(&self) { tokio::time::timeout(Duration::from_secs(30), async { loop { let arrived = self.state.arrived.notified(); @@ -110,12 +106,12 @@ impl MultipartCommitBarrier { .expect("multipart completion should reach the deterministic commit barrier"); } - pub(crate) fn release(&self) { + pub fn release(&self) { self.state.release.add_permits(self.state.expected_arrivals); } } -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] impl Drop for MultipartCommitBarrier { fn drop(&mut self) { self.release(); @@ -129,7 +125,7 @@ impl Drop for MultipartCommitBarrier { } } -#[cfg(test)] +#[cfg(any(test, feature = "test-util"))] async fn pause_multipart_commit(bucket: &str, object: &str, pause: MultipartCommitPause) { let barrier = MULTIPART_COMMIT_BARRIER .get_or_init(|| std::sync::Mutex::new(None)) @@ -1761,8 +1757,16 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { fi.parts = Vec::with_capacity(uploaded_parts.len()); - let quota_context = - reservation::begin(&self.ctx, bucket, object, opts.quota_admission, self.pool_index, self.set_index).await?; + let quota_context = reservation::begin( + &self.ctx, + bucket, + object, + opts.quota_admission, + opts.data_movement, + self.pool_index, + self.set_index, + ) + .await?; let quota_mutation_fence = quota_context.is_enforced() || opts.quota_admission.is_some(); let preserve_replication_ciphertext = opts.replication_request && contains_key_str(&fi.metadata, rustfs_utils::http::SUFFIX_REPLICATION_PRESERVE_CIPHERTEXT); @@ -2125,7 +2129,11 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { } let quota_old_size = if quota_context.is_enforced() { - reservation::replaced_logical_size(&self, bucket, object, opts).await? + if opts.data_movement { + quota_new_size + } else { + reservation::replaced_logical_size(&self, bucket, object, opts).await? + } } else { 0 }; @@ -2228,7 +2236,10 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { .await; return Err(err); } + #[cfg(any(test, feature = "test-util"))] + pause_multipart_commit(bucket, object, MultipartCommitPause::BeforeQuotaRename).await; if quota_reservation.is_lock_lost() + || !quota_reservation.capability_proof_matches() || object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || upload_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || opts diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index 8095f8cdb..349083cc5 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -62,6 +62,10 @@ fn duration_millis_f64(duration: std::time::Duration) -> f64 { duration.as_secs_f64() * 1000.0 } +fn committed_response_metadata_slot(committed_disks: &[Option], fallback_slot: usize) -> usize { + committed_disks.iter().position(Option::is_some).unwrap_or(fallback_slot) +} + #[cfg(test)] mod duration_metrics_tests { use super::duration_millis_f64; @@ -73,6 +77,38 @@ mod duration_metrics_tests { } } +#[cfg(test)] +mod put_metadata_tests { + use super::*; + + #[test] + fn committed_file_info_follows_exact_quorum_success_slot() { + let mut first_success = FileInfo::new("bucket/object", 2, 2); + first_success.name = "first-success".to_string(); + let mut second_success = first_success.clone(); + second_success.name = "second-success".to_string(); + let mut parts_metadata = [FileInfo::default(), first_success, second_success, FileInfo::default()]; + let committed_disks = [None, Some(()), Some(()), None]; + + assert_eq!( + committed_disks.iter().filter(|disk| disk.is_some()).count(), + 2, + "fixture must meet exact quorum" + ); + let selected_slot = committed_response_metadata_slot(&committed_disks, 3); + let selected = std::mem::take(&mut parts_metadata[selected_slot]); + + assert_eq!(selected.name, "first-success"); + assert_eq!(parts_metadata[1], FileInfo::default(), "selected metadata should move without cloning"); + assert_eq!(parts_metadata[2].name, "second-success", "other committed metadata must remain available"); + assert_eq!( + committed_response_metadata_slot::<()>(&[None, None, None, None], 3), + 3, + "a violated post-commit success-mask invariant must not turn a durable PUT into an error" + ); + } +} + fn is_restore_control_metadata(key: &str) -> bool { key.eq_ignore_ascii_case(X_AMZ_RESTORE.as_str()) || key.eq_ignore_ascii_case(rustfs_utils::http::headers::AMZ_RESTORE_EXPIRY_DAYS) @@ -1060,10 +1096,7 @@ impl SetDisks { } fi.data_dir = Some(Uuid::new_v4()); - - let parts_metadata = vec![fi.clone(); disks.len()]; - - let (mut shuffle_disks, mut parts_metadatas) = Self::shuffle_disks_and_parts_metadata(&disks, &parts_metadata, &fi); + let mut shuffle_disks = Self::shuffle_disks_owned(disks, &fi.erasure.distribution); let tmp_dir = Uuid::new_v4().to_string(); @@ -1075,61 +1108,91 @@ impl SetDisks { let put_object_size = known_put_object_storage_size(data.size()); let is_inline_buffer = storage_class_config.should_inline(erasure.shard_file_size(put_object_size), opts.versioned); + let collect_stage_timing = rustfs_io_metrics::put_stage_metrics_enabled() || issue3031_diag_enabled(); let shard_file_size = erasure.shard_file_size(put_object_size); let shard_size = erasure.shard_size(); - let writer_setup_stage_start = Instant::now(); - let writer_futs: Vec<_> = shuffle_disks - .iter() - .map(|disk_op| { - let tmp_obj = tmp_object.clone(); - async move { - if let Some(disk) = disk_op - && disk.is_online().await - { - match create_bitrot_writer( - is_inline_buffer, - Some(disk), - RUSTFS_META_TMP_BUCKET, - &tmp_obj, - shard_file_size, - shard_size, - HashAlgorithm::HighwayHash256S, - ) - .await - { - Ok(writer) => (Some(writer), None), - Err(err) => { - warn!( - event = EVENT_SET_DISK_WRITE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_SET_DISK, - disk = ?disk, - state = "bitrot_writer_skipped", - error = ?err, - "Set disk bitrot writer skipped" - ); - (None, Some(err)) - } - } - } else { - (None, Some(DiskError::DiskNotFound)) - } + let write_path = classify_put_write_path(is_inline_buffer, put_object_size, fi.erasure.block_size); + let direct_inline_commit = matches!(write_path, SmallWritePath::Inline); + rustfs_io_metrics::record_put_object_path(write_path.metric_label()); + let writer_setup_stage_start = collect_stage_timing.then(Instant::now); + let (mut writers, errors) = if direct_inline_commit { + let online = join_all(shuffle_disks.iter().map(|disk| async move { + if let Some(disk) = disk { + disk.is_online().await + } else { + false } - }) - .collect(); - let writer_results = join_all(writer_futs).await; - let mut writers = Vec::with_capacity(writer_results.len()); - let mut errors = Vec::with_capacity(writer_results.len()); - for (w, e) in writer_results { - writers.push(w); - errors.push(e); + })) + .await; + let mut errors = Vec::with_capacity(online.len()); + for (disk, is_online) in shuffle_disks.iter_mut().zip(online) { + if is_online { + errors.push(None); + } else { + *disk = None; + errors.push(Some(DiskError::DiskNotFound)); + } + } + (std::iter::repeat_with(|| None).take(shuffle_disks.len()).collect(), errors) + } else { + let writer_futs: Vec<_> = shuffle_disks + .iter() + .map(|disk_op| { + let tmp_obj = tmp_object.clone(); + async move { + if let Some(disk) = disk_op + && disk.is_online().await + { + match create_bitrot_writer( + is_inline_buffer, + Some(disk), + RUSTFS_META_TMP_BUCKET, + &tmp_obj, + shard_file_size, + shard_size, + HashAlgorithm::HighwayHash256S, + ) + .await + { + Ok(writer) => (Some(writer), None), + Err(err) => { + warn!( + event = EVENT_SET_DISK_WRITE, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_SET_DISK, + disk = ?disk, + state = "bitrot_writer_skipped", + error = ?err, + "Set disk bitrot writer skipped" + ); + (None, Some(err)) + } + } + } else { + (None, Some(DiskError::DiskNotFound)) + } + } + }) + .collect(); + let writer_results = join_all(writer_futs).await; + let mut writers = Vec::with_capacity(writer_results.len()); + let mut errors = Vec::with_capacity(writer_results.len()); + for (writer, error) in writer_results { + writers.push(writer); + errors.push(error); + } + (writers, errors) + }; + let writer_setup_elapsed = writer_setup_stage_start.map(|stage_start| stage_start.elapsed()); + let writer_setup_ms = writer_setup_elapsed + .map(|elapsed| elapsed.as_millis() as u64) + .unwrap_or_default(); + if let Some(writer_setup_elapsed) = writer_setup_elapsed { + rustfs_io_metrics::record_put_object_stage_duration( + "set_disk_writer_setup", + duration_millis_f64(writer_setup_elapsed), + ); } - let writer_setup_elapsed = writer_setup_stage_start.elapsed(); - let writer_setup_ms = writer_setup_elapsed.as_millis() as u64; - rustfs_io_metrics::record_put_object_stage_duration( - "set_disk_writer_setup", - duration_millis_f64(writer_setup_elapsed), - ); let nil_count = errors.iter().filter(|&e| e.is_none()).count(); if nil_count < write_quorum { @@ -1157,21 +1220,23 @@ impl SetDisks { HashReader::from_stream(Cursor::new(Vec::new()), 0, 0, None, None, false)?, ); - let write_path = classify_put_write_path(is_inline_buffer, put_object_size, fi.erasure.block_size); - rustfs_io_metrics::record_put_object_path(write_path.metric_label()); let small_size_hint = if matches!(write_path, SmallWritePath::Inline | SmallWritePath::SingleBlockNonInline) { usize::try_from(put_object_size).map_err(Error::other)? } else { 0 }; - let encode_stage_start = Instant::now(); + let encode_stage_start = collect_stage_timing.then(Instant::now); + let mut inline_shards = None; let (reader, w_size) = match write_path { SmallWritePath::Inline => match Arc::clone(&erasure) - .encode_inline_small_with_size_hint(stream, &mut writers, write_quorum, small_size_hint) + .encode_inline_shards_with_size_hint(stream, small_size_hint) .await { - Ok((r, w)) => (r, w), + Ok((r, w, shards)) => { + inline_shards = Some(shards); + (r, w) + } Err(e) => { error!("encode_inline_small err {:?}", e); return Err(e.into()); @@ -1204,9 +1269,11 @@ impl SetDisks { } }, }; - let encode_elapsed = encode_stage_start.elapsed(); - let encode_ms = encode_elapsed.as_millis() as u64; - rustfs_io_metrics::record_put_object_stage_duration("set_disk_encode", duration_millis_f64(encode_elapsed)); + let encode_elapsed = encode_stage_start.map(|stage_start| stage_start.elapsed()); + let encode_ms = encode_elapsed.map(|elapsed| elapsed.as_millis() as u64).unwrap_or_default(); + if let Some(encode_elapsed) = encode_elapsed { + rustfs_io_metrics::record_put_object_stage_duration("set_disk_encode", duration_millis_f64(encode_elapsed)); + } let _ = mem::replace(&mut data.stream, reader); // if let Err(err) = close_bitrot_writers(&mut writers).await { @@ -1292,37 +1359,67 @@ impl SetDisks { // drop it below reconstructable quorum (backlog#852 / #799 B3). // `rename_data` re-checks write quorum over the surviving disks and // rolls back if too few remain. - let committed_shards = drop_failed_writer_disks(&mut shuffle_disks, &writers); + let committed_shards = if matches!(write_path, SmallWritePath::Inline) { + shuffle_disks.iter().filter(|disk| disk.is_some()).count() + } else { + drop_failed_writer_disks(&mut shuffle_disks, &writers) + }; if committed_shards < write_quorum { return Err(Error::other(format!( "put_object write quorum unavailable after encode: {committed_shards} shard(s) committed, need {write_quorum}" ))); } - for (i, pfi) in parts_metadatas.iter_mut().enumerate() { - pfi.metadata = user_defined.clone(); + fi.metadata = user_defined; + fi.mod_time = mod_time; + fi.size = w_size as i64; + fi.versioned = opts.versioned || opts.version_suspended; + fi.add_object_part(1, etag, w_size, mod_time, actual_size, index_op, None); + if opts.data_movement { + fi.set_data_moved(); + } + let parity_blocks = fi.erasure.parity_blocks; + + let response_metadata_slot = shuffle_disks + .iter() + .rposition(Option::is_some) + .ok_or_else(|| Error::other("put_object write quorum unavailable after encode"))?; + let mut base_file_info = fi; + let mut parts_metadatas = Vec::with_capacity(shuffle_disks.len()); + for (i, disk) in shuffle_disks.iter().enumerate() { + if disk.is_none() { + parts_metadatas.push(FileInfo::default()); + continue; + } + + let mut pfi = if i == response_metadata_slot { + std::mem::take(&mut base_file_info) + } else { + base_file_info.clone() + }; if is_inline_buffer { - if let Some(writer) = writers[i].take() { + if let Some(shards) = inline_shards.as_ref() { + pfi.data = Some( + shards + .get(i) + .cloned() + .ok_or_else(|| Error::other(format!("inline encoder omitted disk shard {i}")))?, + ); + } else if let Some(writer) = writers[i].take() { pfi.data = Some(writer.into_inline_data().map(Bytes::from).unwrap_or_default()); } pfi.set_inline_data(); } - - pfi.mod_time = mod_time; - pfi.size = w_size as i64; - pfi.versioned = opts.versioned || opts.version_suspended; - pfi.add_object_part(1, etag.clone(), w_size, mod_time, actual_size, index_op.clone(), None); - pfi.checksum = fi.checksum.clone(); - - if opts.data_movement { - pfi.set_data_moved(); - } + parts_metadatas.push(pfi); } + let committed_version_id = parts_metadatas[response_metadata_slot].version_id; + let committed_data_dir = parts_metadatas[response_metadata_slot].data_dir; + let is_compressed = parts_metadatas[response_metadata_slot].is_compressed(); drop(writers); // drop writers to close all files, this is to prevent FileAccessDenied errors when renaming data - if fi.erasure.parity_blocks == 0 { + if parity_blocks == 0 { let written_size = i64::try_from(w_size).map_err(|_| Error::other("put_object written size overflows i64"))?; let logical_shard_size = usize::try_from(erasure.shard_file_size(written_size)) .map_err(|_| Error::other("put_object shard size overflows usize"))?; @@ -1496,8 +1593,16 @@ impl SetDisks { }); } - let quota_context = - reservation::begin(&self.ctx, bucket, object, opts.quota_admission, self.pool_index, self.set_index).await?; + let quota_context = reservation::begin( + &self.ctx, + bucket, + object, + opts.quota_admission, + opts.data_movement, + self.pool_index, + self.set_index, + ) + .await?; let quota_mutation_fence = quota_context.is_enforced() || opts.quota_admission.is_some(); let mut replication_quota_size = None; @@ -1506,17 +1611,16 @@ impl SetDisks { return Err(Error::PartMissingOrCorrupt); } if quota_context.is_enforced() { + let persisted_metadata = &parts_metadatas[response_metadata_slot].metadata; let observed_size = u64::try_from(actual_size).map_err(|_| Error::PartMissingOrCorrupt)?; let physical_size = u64::try_from(w_size).map_err(|_| Error::PartMissingOrCorrupt)?; - let transformed = contains_key_str(&user_defined, SUFFIX_COMPRESSION) - || parts_metadatas - .first() - .is_some_and(|metadata| should_persist_encryption_original_size(&metadata.metadata)); - let declared_size = get_str(&user_defined, SUFFIX_ACTUAL_SIZE) + let transformed = contains_key_str(persisted_metadata, SUFFIX_COMPRESSION) + || should_persist_encryption_original_size(persisted_metadata); + let declared_size = get_str(persisted_metadata, SUFFIX_ACTUAL_SIZE) .map(|value| value.parse::().map_err(|_| Error::PartMissingOrCorrupt)) .transpose()? .unwrap_or(0); - let declared_encryption_size = rustfs_utils::http::get_object_encryption_original_size(&user_defined) + let declared_encryption_size = rustfs_utils::http::get_object_encryption_original_size(persisted_metadata) .map_err(Error::other)? .map(u64::try_from) .transpose() @@ -1544,10 +1648,9 @@ impl SetDisks { } } else if actual_size >= 0 { let observed_size = u64::try_from(actual_size).map_err(|_| Error::PartMissingOrCorrupt)?; - let transformed = contains_key_str(&user_defined, SUFFIX_COMPRESSION) - || parts_metadatas - .first() - .is_some_and(|metadata| should_persist_encryption_original_size(&metadata.metadata)); + let persisted_metadata = &parts_metadatas[response_metadata_slot].metadata; + let transformed = contains_key_str(persisted_metadata, SUFFIX_COMPRESSION) + || should_persist_encryption_original_size(persisted_metadata); let server_observed_size = if transformed { observed_size } else { @@ -1574,7 +1677,12 @@ impl SetDisks { .map_err(|_| Error::PartMissingOrCorrupt)? .max(u64::try_from(w_size).map_err(|_| Error::PartMissingOrCorrupt)?), }; - (reservation::replaced_logical_size(self, bucket, object, opts).await?, new_size) + let old_size = if opts.data_movement { + new_size + } else { + reservation::replaced_logical_size(self, bucket, object, opts).await? + }; + (old_size, new_size) } else { (0, 0) }; @@ -1674,6 +1782,7 @@ impl SetDisks { #[cfg(any(test, feature = "test-util"))] pause_put_object_commit(bucket, object, PutObjectCommitPause::BeforeQuotaRename).await; if quota_reservation.is_lock_lost() + || !quota_reservation.capability_proof_matches() || object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || opts .namespace_lock_fence @@ -1736,7 +1845,7 @@ impl SetDisks { Some(self.pool_index), Some(self.set_index), ); - request.object_version_id = fi.version_id.map(|version_id| version_id.to_string()); + request.object_version_id = committed_version_id.map(|version_id| version_id.to_string()); tokio::spawn(async move { let _ = rustfs_common::heal_channel::send_heal_request(request).await; }); @@ -1771,7 +1880,7 @@ impl SetDisks { let mut cleanup_stage_ms: Option = None; if let Some(old_dir) = op_old_dir { - let committed_dir = fi.data_dir.unwrap_or_default().to_string(); + let committed_dir = committed_data_dir.unwrap_or_default().to_string(); let cleanup_stage_start = Instant::now(); // backlog#898: reclaiming the dereferenced old data dir is // best-effort and returns a receipt (never `Err`). A failed GC @@ -1808,16 +1917,10 @@ impl SetDisks { } } - for (i, op_disk) in online_disks.iter().enumerate() { - if let Some(disk) = op_disk - && disk.is_online().await - { - fi = parts_metadatas[i].clone(); - break; - } - } + let committed_metadata_slot = committed_response_metadata_slot(&online_disks, response_metadata_slot); + let mut fi = std::mem::take(&mut parts_metadatas[committed_metadata_slot]); - if fi.is_compressed() { + if is_compressed { record_compression_total_memory(actual_size as u64, w_size as u64).await; } self.record_capacity_scope_if_needed(opts.capacity_scope_token, &online_disks); @@ -5906,6 +6009,264 @@ mod replication_quota_safety_tests { } } +#[cfg(test)] +mod inline_put_commit_path_tests { + use super::hermetic_set_disks_support::hermetic_set_disks_isolated as hermetic_set_disks; + use super::*; + use crate::disk::{DiskAPI as _, ReadOptions}; + use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _}; + use tokio::io::AsyncReadExt; + + async fn make_bucket(disks: &[DiskStore], bucket: &str) { + for disk in disks { + disk.make_volume(bucket).await.expect("bucket volume should be created"); + } + } + + #[tokio::test] + async fn inline_put_direct_commit_round_trips_verified_bitrot_shards() { + let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await; + let bucket = "inline-direct-commit"; + let object = "object.bin"; + let payload: Vec = (0..16 * 1024).map(|index| (index % 251) as u8).collect(); + make_bucket(&disk_stores, bucket).await; + + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &ObjectOptions::default()) + .await + .expect("inline PUT should commit"); + + let read_data = ReadOptions { + read_data: true, + ..Default::default() + }; + for (disk_index, disk) in disk_stores.iter().enumerate() { + let file_info = disk + .read_version("", bucket, object, "", &read_data) + .await + .unwrap_or_else(|err| panic!("disk {disk_index} should persist inline metadata: {err}")); + assert!(file_info.inline_data(), "disk {disk_index} should mark the shard inline"); + let inline_data = file_info + .data + .as_ref() + .unwrap_or_else(|| panic!("disk {disk_index} should persist inline bitrot bytes")); + let erasure = erasure_from_file_info(&file_info, false).expect("persisted erasure layout should be valid"); + let logical_shard_size = + usize::try_from(erasure.shard_file_size(payload.len() as i64)).expect("logical shard size should fit usize"); + coding::bitrot_verify( + Cursor::new(inline_data.clone()), + inline_data.len(), + logical_shard_size, + HashAlgorithm::HighwayHash256S, + erasure.shard_size(), + ) + .await + .unwrap_or_else(|err| panic!("disk {disk_index} inline shard should pass bitrot verification: {err}")); + } + + let mut object_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &ObjectOptions::default()) + .await + .expect("committed inline object should be readable"); + let mut restored = Vec::new(); + object_reader + .stream + .read_to_end(&mut restored) + .await + .expect("inline object should stream"); + assert_eq!(restored, payload); + } + + #[tokio::test] + async fn inline_put_direct_commit_accepts_exact_quorum_and_rejects_quorum_minus_one() { + let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await; + let bucket = "inline-direct-quorum"; + let exact_quorum_object = "exact-quorum.bin"; + let below_quorum_object = "below-quorum.bin"; + make_bucket(&disk_stores, bucket).await; + { + let mut disks = set_disks.disks.write().await; + disks[3] = None; + } + + let mut reader = PutObjReader::from_vec(vec![0x5a; 4 * 1024]); + set_disks + .put_object(bucket, exact_quorum_object, &mut reader, &ObjectOptions::default()) + .await + .expect("three online disks should satisfy the four-disk write quorum"); + for (disk_index, disk) in disk_stores.iter().enumerate() { + let persisted = disk + .read_version("", bucket, exact_quorum_object, "", &ReadOptions::default()) + .await; + assert_eq!( + persisted.is_ok(), + disk_index < 3, + "exact-quorum commit should publish only on the three online disks" + ); + } + + set_disks.disks.write().await[2] = None; + let mut reader = PutObjReader::from_vec(vec![0xa5; 4 * 1024]); + set_disks + .put_object(bucket, below_quorum_object, &mut reader, &ObjectOptions::default()) + .await + .expect_err("two online disks are one below the four-disk write quorum"); + + for (disk_index, disk) in disk_stores.iter().enumerate() { + assert!( + disk.read_version("", bucket, below_quorum_object, "", &ReadOptions::default()) + .await + .is_err(), + "disk {disk_index} must not expose an object after pre-commit quorum failure" + ); + } + } + + #[tokio::test] + async fn inline_put_direct_commit_handles_post_encode_rename_failures() { + use crate::disk::health_state::RuntimeDriveHealthState; + + let payload = vec![0x5a; 4 * 1024]; + + let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await; + let bucket = "inline-direct-post-encode-quorum"; + let object = "exact-quorum.bin"; + make_bucket(&disk_stores, bucket).await; + let barrier = PutObjectCommitBarrier::install(bucket, object, PutObjectCommitPause::AfterNamespace); + let put = { + let set_disks = Arc::clone(&set_disks); + let payload = payload.clone(); + tokio::spawn(async move { + let mut reader = PutObjReader::from_vec(payload); + set_disks + .put_object(bucket, object, &mut reader, &ObjectOptions::default()) + .await + }) + }; + barrier.wait_until_paused().await; + disk_stores[3].force_runtime_state_for_test(RuntimeDriveHealthState::Offline); + barrier.release(); + put.await + .expect("exact-quorum PUT task should complete") + .expect("one post-encode rename failure should preserve write quorum"); + disk_stores[3].force_runtime_state_for_test(RuntimeDriveHealthState::Online); + for (disk_index, disk) in disk_stores.iter().enumerate() { + let persisted = disk.read_version("", bucket, object, "", &ReadOptions::default()).await; + assert_eq!( + persisted.is_ok(), + disk_index < 3, + "only disks that completed rename_data may publish the exact-quorum object" + ); + } + + let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await; + let bucket = "inline-direct-post-encode-rollback"; + let object = "rollback.bin"; + let old_payload = vec![0x31; 4 * 1024]; + make_bucket(&disk_stores, bucket).await; + let mut old_reader = PutObjReader::from_vec(old_payload.clone()); + set_disks + .put_object(bucket, object, &mut old_reader, &ObjectOptions::default()) + .await + .expect("old inline object should commit"); + let read_data = ReadOptions { + read_data: true, + ..Default::default() + }; + let mut old_disk_data = Vec::with_capacity(disk_stores.len()); + for disk in &disk_stores { + old_disk_data.push( + disk.read_version("", bucket, object, "", &read_data) + .await + .expect("old inline shard should be readable before overwrite") + .data, + ); + } + + let barrier = PutObjectCommitBarrier::install(bucket, object, PutObjectCommitPause::AfterNamespace); + let put = { + let set_disks = Arc::clone(&set_disks); + let payload = payload.clone(); + tokio::spawn(async move { + let mut reader = PutObjReader::from_vec(payload); + set_disks + .put_object(bucket, object, &mut reader, &ObjectOptions::default()) + .await + }) + }; + barrier.wait_until_paused().await; + for disk in &disk_stores[2..] { + disk.force_runtime_state_for_test(RuntimeDriveHealthState::Offline); + } + barrier.release(); + put.await + .expect("quorum-minus-one PUT task should complete") + .expect_err("two post-encode rename failures must fail write quorum"); + for disk in &disk_stores[2..] { + disk.force_runtime_state_for_test(RuntimeDriveHealthState::Online); + } + + for (disk_index, disk) in disk_stores.iter().enumerate() { + let restored = disk + .read_version("", bucket, object, "", &read_data) + .await + .unwrap_or_else(|err| panic!("disk {disk_index} should retain the old inline object: {err}")); + assert_eq!(restored.data, old_disk_data[disk_index]); + } + let mut object_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &ObjectOptions::default()) + .await + .expect("old object should remain readable after quorum rollback"); + let mut restored = Vec::new(); + object_reader + .stream + .read_to_end(&mut restored) + .await + .expect("old object should stream after quorum rollback"); + assert_eq!(restored, old_payload); + } + + #[tokio::test] + async fn zero_length_put_keeps_existing_pipeline_layout_and_round_trips() { + let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await; + let bucket = "zero-length-put"; + let object = "empty.bin"; + make_bucket(&disk_stores, bucket).await; + + let mut reader = PutObjReader::from_vec(Vec::new()); + set_disks + .put_object(bucket, object, &mut reader, &ObjectOptions::default()) + .await + .expect("zero-length PUT should commit through the existing pipeline"); + + let read_data = ReadOptions { + read_data: true, + ..Default::default() + }; + for (disk_index, disk) in disk_stores.iter().enumerate() { + let file_info = disk + .read_version("", bucket, object, "", &read_data) + .await + .unwrap_or_else(|err| panic!("disk {disk_index} should persist empty-object metadata: {err}")); + assert_eq!(file_info.size, 0); + assert_eq!(file_info.data.as_deref(), Some(&[][..])); + } + + let mut object_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &ObjectOptions::default()) + .await + .expect("empty object should be readable"); + let mut restored = Vec::new(); + object_reader + .stream + .read_to_end(&mut restored) + .await + .expect("empty object should stream"); + assert!(restored.is_empty()); + } +} + #[cfg(test)] mod get_object_downstream_close_accounting_tests { use super::hermetic_set_disks_support::hermetic_set_disks; diff --git a/crates/io-metrics/src/lib.rs b/crates/io-metrics/src/lib.rs index 6d2df2b9c..658dee400 100644 --- a/crates/io-metrics/src/lib.rs +++ b/crates/io-metrics/src/lib.rs @@ -58,7 +58,7 @@ use std::sync::{ /// When `false`, `record_put_object_path` and `record_put_object_stage_duration` /// become no-ops, and callers can skip the `Instant::now()` syscalls entirely. /// -/// Set to `true` during startup when OTEL metric export is enabled. +/// Enabled only through an explicit runtime opt-in. static PUT_STAGE_METRICS_ENABLED: AtomicBool = AtomicBool::new(false); static GET_STAGE_METRICS_ENABLED: AtomicBool = AtomicBool::new(false); @@ -78,7 +78,7 @@ static METRICS_ENABLED: AtomicBool = AtomicBool::new(false); /// Enable or disable detailed per-stage PUT metrics. /// -/// Called once during startup, typically gated by `rustfs_obs::observability_metric_enabled()`. +/// Called once during startup after applying the detailed PUT attribution opt-in. pub fn set_put_stage_metrics_enabled(enabled: bool) { PUT_STAGE_METRICS_ENABLED.store(enabled, Ordering::Relaxed); } @@ -103,6 +103,12 @@ pub fn put_stage_metrics_enabled() -> bool { PUT_STAGE_METRICS_ENABLED.load(Ordering::Relaxed) } +/// Start a PUT-stage timer only when detailed PUT attribution is enabled. +#[inline(always)] +pub fn put_stage_timer() -> Option { + put_stage_metrics_enabled().then(std::time::Instant::now) +} + #[inline(always)] pub fn get_stage_metrics_enabled() -> bool { GET_STAGE_METRICS_ENABLED.load(Ordering::Relaxed) @@ -434,7 +440,7 @@ pub fn record_get_object_request_result(status: &str, duration_secs: f64) { /// Record PutObject request start. #[inline(always)] pub fn record_put_object_request_start(concurrent_requests: usize) { - if !put_stage_metrics_enabled() { + if !metrics_enabled() { return; } counter!("rustfs_io_put_object_requests_total").increment(1); @@ -444,7 +450,7 @@ pub fn record_put_object_request_start(concurrent_requests: usize) { /// Record PutObject request result. #[inline(always)] pub fn record_put_object_request_result(status: &str, duration_secs: f64) { - if !put_stage_metrics_enabled() { + if !metrics_enabled() { return; } counter!("rustfs_io_put_object_request_results_total", "status" => status.to_string()).increment(1); @@ -1905,7 +1911,7 @@ pub fn record_get_object(duration_ms: f64, size_bytes: i64) { /// * `zero_copy_eligible` - Whether the request was eligible for a zero-copy path #[inline(always)] pub fn record_put_object(duration_ms: f64, size_bytes: i64, zero_copy_eligible: bool) { - if !put_stage_metrics_enabled() { + if !metrics_enabled() { return; } counter!("rustfs_s3_put_object_total").increment(1); @@ -2004,6 +2010,13 @@ pub fn record_put_object_stage_duration(stage: &'static str, duration_ms: f64) { histogram!("rustfs_s3_put_object_stage_duration_ms", "stage" => stage).record(duration_ms); } +#[inline(always)] +pub fn record_put_object_stage_duration_from(stage: &'static str, started_at: Option) { + if let Some(started_at) = started_at { + record_put_object_stage_duration(stage, started_at.elapsed().as_secs_f64() * 1000.0); + } +} + /// Record generic internal operation stage duration (non-PUT paths). /// Use this for metacache walks, listing, lifecycle, and other background /// operations that are NOT part of the PUT object hot path. @@ -2819,6 +2832,56 @@ mod tests { assert!(!put_stage_metrics_enabled()); } + #[test] + fn put_stage_gate_does_not_disable_basic_put_metrics() { + let _guard = METRICS_FLAG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + set_metrics_enabled(true); + set_put_stage_metrics_enabled(false); + record_put_object_request_start(1); + record_put_object_request_result("ok", 0.001); + record_put_object(1.0, 1024, false); + record_put_object_stage_duration("disabled_stage", 0.5); + + set_put_stage_metrics_enabled(true); + record_put_object_stage_duration("enabled_stage", 0.5); + + set_put_stage_metrics_enabled(false); + set_metrics_enabled(false); + }); + + let metrics = snapshotter.snapshot().into_vec(); + assert!(metrics.iter().any(|(composite, _, _, _)| { + composite.kind() == MetricKind::Counter && composite.key().name() == "rustfs_s3_put_object_total" + })); + assert!(metrics.iter().any(|(composite, _, _, _)| { + composite.kind() == MetricKind::Counter && composite.key().name() == "rustfs_io_put_object_requests_total" + })); + + let stages = metrics + .iter() + .filter(|(composite, _, _, _)| { + composite.kind() == MetricKind::Histogram && composite.key().name() == "rustfs_s3_put_object_stage_duration_ms" + }) + .flat_map(|(composite, _, _, _)| composite.key().labels().map(|label| label.value().to_string())) + .collect::>(); + assert_eq!(stages, ["enabled_stage"]); + } + + #[test] + fn test_put_stage_timer_follows_metrics_switch() { + let _guard = METRICS_FLAG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + set_put_stage_metrics_enabled(false); + assert!(put_stage_timer().is_none()); + + set_put_stage_metrics_enabled(true); + assert!(put_stage_timer().is_some()); + set_put_stage_metrics_enabled(false); + } + #[test] fn test_record_get_object_path_and_stage() { let _guard = METRICS_FLAG_LOCK.lock().unwrap_or_else(|e| e.into_inner()); diff --git a/crates/kms/src/backends/vault.rs b/crates/kms/src/backends/vault.rs index 86caf430f..84ba8e6fa 100644 --- a/crates/kms/src/backends/vault.rs +++ b/crates/kms/src/backends/vault.rs @@ -208,6 +208,7 @@ impl<'de> Deserialize<'de> for VaultKeyData { RotatedAt, EncryptedKeyMaterial, BaselineVersion, + WrapBudgetReserved, Unknown(BoundedUnknownFieldName), } @@ -242,6 +243,7 @@ impl<'de> Deserialize<'de> for VaultKeyData { "rotated_at" => Field::RotatedAt, "encrypted_key_material" => Field::EncryptedKeyMaterial, "baseline_version" => Field::BaselineVersion, + "wrap_budget_reserved" => Field::WrapBudgetReserved, _ => Field::Unknown(BoundedUnknownFieldName::new(value)), }) } @@ -285,6 +287,7 @@ impl<'de> Deserialize<'de> for VaultKeyData { let mut rotated_at = None; let mut encrypted_key_material = None; let mut baseline_version = None; + let mut wrap_budget_reserved = None; let mut unknown_fields = UnknownFieldSummary::default(); while let Some(field) = map.next_key()? { @@ -301,6 +304,7 @@ impl<'de> Deserialize<'de> for VaultKeyData { Field::RotatedAt => read_field!(rotated_at, "rotated_at"), Field::EncryptedKeyMaterial => read_field!(encrypted_key_material, "encrypted_key_material"), Field::BaselineVersion => read_field!(baseline_version, "baseline_version"), + Field::WrapBudgetReserved => read_field!(wrap_budget_reserved, "wrap_budget_reserved"), Field::Unknown(field) => { let _: IgnoredAny = map.next_value()?; unknown_fields.observe(field); @@ -322,6 +326,10 @@ impl<'de> Deserialize<'de> for VaultKeyData { encrypted_key_material: encrypted_key_material .ok_or_else(|| de::Error::missing_field("encrypted_key_material"))?, baseline_version: baseline_version.unwrap_or(None), + // Absent on records written before wrap accounting existed, and + // on records an older build rewrote; zero restarts the + // reservation rather than blocking a wrap. + wrap_budget_reserved: wrap_budget_reserved.unwrap_or(0), }; unknown_fields.record_for_vault_kv2_key(); Ok(key_data) @@ -341,6 +349,7 @@ impl<'de> Deserialize<'de> for VaultKeyData { "rotated_at", "encrypted_key_material", "baseline_version", + "wrap_budget_reserved", ]; deserializer.deserialize_struct("VaultKeyData", FIELDS, VaultKeyDataVisitor) } @@ -3014,6 +3023,54 @@ mod tests { assert_eq!(legacy.version, 1); } + /// Every declared `VaultKeyData` field must survive a serialize/deserialize + /// round trip through the hand-written `Deserialize`. + /// + /// The hand-written impl lists its fields three times (the `Field` enum, the + /// match arms, the struct literal), so a field added to the struct alone + /// compiles on its own branch and only breaks once both branches merge — + /// which is exactly how `wrap_budget_reserved` briefly broke the build. + /// Asserting against the serialized key set makes the deserializer's + /// coverage a test failure rather than a merge-order accident. + #[test] + fn vault_key_data_deserializer_covers_every_serialized_field() { + let mut key_data = healthy_key_data(); + key_data.wrap_budget_reserved = 7_000_000; + key_data.baseline_version = Some(2); + key_data.rotated_at = Some(Zoned::now()); + key_data.deletion_date = Some(Zoned::now()); + key_data.description = Some("described".to_string()); + + let value = serde_json::to_value(&key_data).expect("serialize key data"); + let serialized_fields: Vec = value + .as_object() + .expect("key data serializes to an object") + .keys() + .cloned() + .collect(); + + // Every serialized field must be a known field: an unknown one would be + // counted by the unknown-field observer instead of being read back. + let recorder = metrics_util::debugging::DebuggingRecorder::new(); + let restored: VaultKeyData = + metrics::with_local_recorder(&recorder, || serde_json::from_value(value).expect("round trip")); + assert_eq!( + crate::test_support::unknown_field_metric(&recorder, "vault-kv2-key"), + 0, + "a serialized field was not recognized by the deserializer; fields: {serialized_fields:?}" + ); + + // And every value must survive, not just parse. + assert_eq!(restored.wrap_budget_reserved, key_data.wrap_budget_reserved); + assert_eq!(restored.baseline_version, key_data.baseline_version); + assert_eq!(restored.version, key_data.version); + assert_eq!(restored.status, key_data.status); + assert_eq!(restored.description, key_data.description); + assert_eq!(restored.encrypted_key_material, key_data.encrypted_key_material); + assert!(restored.rotated_at.is_some()); + assert!(restored.deletion_date.is_some()); + } + #[test] fn vault_key_data_unknown_fields_remain_readable_and_are_observed() { // A record written by a newer build carries fields this build does not diff --git a/crates/protocols/src/swift/handler.rs b/crates/protocols/src/swift/handler.rs index 0d9cdc455..b8018ab5a 100644 --- a/crates/protocols/src/swift/handler.rs +++ b/crates/protocols/src/swift/handler.rs @@ -748,9 +748,10 @@ async fn handle_authenticated_request( } for (key, value) in info.user_defined.iter() { - if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } @@ -817,9 +818,10 @@ async fn handle_authenticated_request( // Add custom metadata headers (X-Object-Meta-*) for (key, value) in info.user_defined.iter() { - if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } @@ -856,9 +858,10 @@ async fn handle_authenticated_request( // Add custom metadata headers (X-Object-Meta-*) for (key, value) in info.user_defined.iter() { - if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } @@ -1168,9 +1171,10 @@ async fn handle_object_get( } for (key, value) in info.user_defined.iter() { - if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } @@ -1237,9 +1241,10 @@ async fn handle_object_get( if key == "x-delete-at" { // Add X-Delete-At header directly (not as X-Object-Meta-*) response = response.header("x-delete-at", value.as_str()); - } else if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + } else if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } @@ -1293,9 +1298,10 @@ async fn handle_object_head( if key == "x-delete-at" { // Add X-Delete-At header directly (not as X-Object-Meta-*) response = response.header("x-delete-at", value.as_str()); - } else if key != "content-type" { - let header_name = format!("x-object-meta-{}", key); - response = response.header(header_name, value.as_str()); + } else if key != "content-type" + && let Some(key) = object::swift_response_user_metadata_key(key) + { + response = response.header(format!("x-object-meta-{key}"), value.as_str()); } } diff --git a/crates/protocols/src/swift/object.rs b/crates/protocols/src/swift/object.rs index e871cfba2..7169b152a 100644 --- a/crates/protocols/src/swift/object.rs +++ b/crates/protocols/src/swift/object.rs @@ -84,6 +84,24 @@ fn stored_swift_user_metadata_key(key: &str) -> String { } } +pub(super) fn swift_response_user_metadata_key(key: &str) -> Option<&str> { + if rustfs_utils::http::is_internal_key(key) + || rustfs_utils::http::starts_with_ignore_ascii_case(key, "x-rustfs-encryption-") + || rustfs_utils::http::starts_with_ignore_ascii_case(key, "x-minio-encryption-") + { + return None; + } + if let Some(unescaped) = key.strip_prefix(USER_METADATA_PREFIX) + && (rustfs_utils::http::is_internal_key(unescaped) + || rustfs_utils::http::starts_with_ignore_ascii_case(unescaped, "x-amz-") + || rustfs_utils::http::starts_with_ignore_ascii_case(unescaped, "x-rustfs-encryption-") + || rustfs_utils::http::starts_with_ignore_ascii_case(unescaped, "x-minio-encryption-")) + { + return Some(unescaped); + } + Some(key) +} + fn swift_user_metadata(headers: &HeaderMap) -> Option> { let mut metadata = HashMap::new(); let mut present = false; @@ -1142,6 +1160,19 @@ mod tests { assert_eq!(stored_swift_user_metadata_key("description"), "description"); } + #[test] + fn swift_user_metadata_response_mapping_is_reversible_and_filters_internal_keys() { + assert_eq!( + swift_response_user_metadata_key("x-amz-meta-x-rustfs-internal-actual-size"), + Some("x-rustfs-internal-actual-size") + ); + assert_eq!(swift_response_user_metadata_key("x-amz-meta-x-amz-checksum"), Some("x-amz-checksum")); + assert_eq!(swift_response_user_metadata_key("x-amz-meta-description"), Some("x-amz-meta-description")); + assert_eq!(swift_response_user_metadata_key("description"), Some("description")); + assert_eq!(swift_response_user_metadata_key("x-rustfs-internal-actual-size"), None); + assert_eq!(swift_response_user_metadata_key("x-minio-internal-actual-size"), None); + } + #[test] fn test_validate_object_name_empty() { let result = ObjectKeyMapper::validate_object_name(""); diff --git a/rustfs/src/admin/handlers/bucket_meta.rs b/rustfs/src/admin/handlers/bucket_meta.rs index 1a3c2b383..c73d4c487 100644 --- a/rustfs/src/admin/handlers/bucket_meta.rs +++ b/rustfs/src/admin/handlers/bucket_meta.rs @@ -465,6 +465,16 @@ impl Operation for ImportBucketMetadata { file_contents.push((file_path, content)); } + let durable_quota_import = imported_quota_requires_fleet_proof(&file_contents)?; + let quota_fleet_proof = + if durable_quota_import { + Some(crate::admin::storage_api::acquire_cross_pool_fence_fleet_proof().ok_or_else(|| { + s3_error!(ServiceUnavailable, "durable quota capability is not confirmed across the cluster") + })?) + } else { + None + }; + // Extract bucket names let mut bucket_names = Vec::new(); for (file_path, _) in &file_contents { @@ -707,21 +717,6 @@ impl Operation for ImportBucketMetadata { } BUCKET_QUOTA_CONFIG_FILE => { - if let Err(e) = serde_json::from_slice::(&content) { - warn!( - event = EVENT_ADMIN_BUCKET_META_STATE, - component = LOG_COMPONENT_ADMIN, - subsystem = LOG_SUBSYSTEM_BUCKET_META, - action = "import_bucket_metadata", - result = "config_deserialize_failed", - bucket = %bucket_name, - config_name = %conf_name, - error = %e, - "admin bucket meta state" - ); - continue; - } - let metadata = match bucket_metadatas.get_mut(bucket_name) { Some(m) => m, None => continue, @@ -830,10 +825,6 @@ impl Operation for ImportBucketMetadata { } } - // Persist the assembled metadata to disk. Prior to this, the import only mutated the - // in-memory `bucket_metadatas` map and returned 200, silently dropping every imported - // config. `metadata_sys::update` loads the on-disk metadata, overwrites the given config - // field and saves it, preserving any configs not present in the import archive. for (bucket_name, metadata) in &bucket_metadatas { for (config_file, data) in imported_configs_to_persist(metadata) { let site_replication_item = imported_config_to_site_replication_item(bucket_name, metadata, config_file, &data)?; @@ -842,7 +833,21 @@ impl Operation for ImportBucketMetadata { } else { None }; - if let Err(e) = metadata_sys::update(bucket_name, config_file, data).await { + let persist_result = if config_file == BUCKET_QUOTA_CONFIG_FILE { + let quota: BucketQuota = + serde_json::from_slice(&data).map_err(|e| s3_error!(InvalidRequest, "invalid bucket quota: {e}"))?; + if quota.uses_durable_reservations() { + let proof = quota_fleet_proof.as_ref().ok_or_else(|| { + s3_error!(ServiceUnavailable, "durable quota capability is not confirmed across the cluster") + })?; + metadata_sys::update_quota_if_incarnation(bucket_name, data, metadata.bucket_incarnation_id, proof).await + } else { + metadata_sys::update_if_incarnation(bucket_name, config_file, data, metadata.bucket_incarnation_id).await + } + } else { + metadata_sys::update_if_incarnation(bucket_name, config_file, data, metadata.bucket_incarnation_id).await + }; + if let Err(e) = persist_result { warn!( event = EVENT_ADMIN_BUCKET_META_STATE, component = LOG_COMPONENT_ADMIN, @@ -885,6 +890,26 @@ impl Operation for ImportBucketMetadata { } } +fn imported_quota_requires_fleet_proof(file_contents: &[(String, Vec)]) -> S3Result { + let mut durable = false; + for (file_path, content) in file_contents { + let mut parts = file_path.split(SLASH_SEPARATOR); + let Some(_bucket) = parts.next() else { + continue; + }; + if parts.next() != Some(BUCKET_QUOTA_CONFIG_FILE) { + continue; + } + let quota: BucketQuota = + serde_json::from_slice(content).map_err(|e| s3_error!(InvalidRequest, "invalid bucket quota: {e}"))?; + if quota.has_unsupported_reservation_protocol() { + return Err(s3_error!(InvalidRequest, "unsupported bucket quota reservation protocol")); + } + durable |= quota.uses_durable_reservations(); + } + Ok(durable) +} + /// The `(config_file, data)` pairs to persist for an imported bucket's metadata: every non-empty /// config field keyed by its on-disk config-file name, as owned data ready for /// `metadata_sys::update`. Empty fields are skipped so an import never overwrites an existing @@ -1076,4 +1101,31 @@ mod import_persist_tests { assert!(!has_site_replication_item); } + + #[test] + fn quota_import_preflight_rejects_invalid_and_unknown_protocols() { + let missing_limit = vec![( + format!("bucket/{BUCKET_QUOTA_CONFIG_FILE}"), + br#"{"quota":0,"reservation_protocol":1}"#.to_vec(), + )]; + assert!(imported_quota_requires_fleet_proof(&missing_limit).is_err()); + + let unknown_protocol = vec![( + format!("bucket/{BUCKET_QUOTA_CONFIG_FILE}"), + br#"{"quota":0,"reservation_protocol":2,"reservation_quota":1024}"#.to_vec(), + )]; + assert!(imported_quota_requires_fleet_proof(&unknown_protocol).is_err()); + } + + #[test] + fn quota_import_preflight_requires_proof_only_for_durable_quota() { + let legacy = vec![(format!("bucket/{BUCKET_QUOTA_CONFIG_FILE}"), br#"{"quota":1024}"#.to_vec())]; + assert!(!imported_quota_requires_fleet_proof(&legacy).expect("legacy quota should remain compatible")); + + let durable = vec![( + format!("bucket/{BUCKET_QUOTA_CONFIG_FILE}"), + serde_json::to_vec(&BucketQuota::new(Some(1024))).expect("durable quota should encode"), + )]; + assert!(imported_quota_requires_fleet_proof(&durable).expect("durable quota should pass preflight")); + } } diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index c97590c4f..35c4d3f2a 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -2166,6 +2166,101 @@ mod tests { assert_eq!(denied.code(), &S3ErrorCode::InvalidRequest); } + #[tokio::test] + #[serial_test::serial] + async fn multipart_completion_rejects_rotated_quota_capability_before_rename() { + use crate::app::storage_api::test::set_disk::{MultipartCommitBarrier, MultipartCommitPause}; + + let (store, bucket) = crate::app::gating_test_env::durable_quota_test_bucket("rotated-proof-mpu-quota", 4096).await; + let object = "object"; + let upload = store + .new_multipart_upload(&bucket, object, &ObjectOptions::default()) + .await + .expect("create multipart upload"); + let mut reader = PutObjReader::from_vec(vec![0x78; 4096]); + let part = store + .put_object_part(&bucket, object, &upload.upload_id, 1, &mut reader, &ObjectOptions::default()) + .await + .expect("stage multipart part"); + let barrier = MultipartCommitBarrier::install(&bucket, object, MultipartCommitPause::BeforeQuotaRename); + let complete_store = Arc::clone(&store); + let complete_bucket = bucket.clone(); + let upload_id = upload.upload_id.clone(); + let complete = tokio::spawn(async move { + complete_store + .complete_multipart_upload( + &complete_bucket, + object, + &upload_id, + vec![CompletePart { + part_num: 1, + etag: part.etag, + ..Default::default() + }], + &ObjectOptions::default(), + ) + .await + }); + barrier.wait_until_paused().await; + assert!( + crate::storage::storage_api::ecstore_notification::rotate_cross_pool_fence_fleet_proof_for_test(), + "the gating environment must have a current fleet proof" + ); + barrier.release(); + + let err = complete + .await + .expect("completion task should not panic") + .expect_err("a replaced fleet proof must fence multipart rename"); + assert!(matches!( + err, + StorageError::NamespaceLockQuorumUnavailable { + mode: "quota_reservation", + .. + } + )); + store + .get_multipart_info(&bucket, object, &upload.upload_id, &ObjectOptions::default()) + .await + .expect("proof rotation must preserve the multipart upload for retry"); + } + + #[tokio::test] + #[serial_test::serial] + async fn data_movement_multipart_completion_has_zero_quota_growth() { + let (store, bucket) = crate::app::gating_test_env::durable_quota_test_bucket("data-movement-mpu-quota", 0).await; + let object = "object"; + let mut movement_opts = ObjectOptions { + data_movement: true, + ..Default::default() + }; + let upload = store + .new_multipart_upload(&bucket, object, &movement_opts) + .await + .expect("create data-movement multipart upload"); + let mut reader = PutObjReader::from_vec(vec![0x7a; 4096]); + let part = store + .put_object_part(&bucket, object, &upload.upload_id, 1, &mut reader, &movement_opts) + .await + .expect("stage data-movement multipart part"); + movement_opts.preserve_etag = Some("movement-etag".to_string()); + let completed = store + .complete_multipart_upload( + &bucket, + object, + &upload.upload_id, + vec![CompletePart { + part_num: 1, + etag: part.etag, + ..Default::default() + }], + &movement_opts, + ) + .await + .expect("moving an already-accounted multipart object between pools must have zero quota growth"); + assert_eq!(completed.size, 4096); + } + #[tokio::test] #[serial_test::serial] async fn rejected_empty_parts_preserve_existing_object_and_staging() { diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 4f665b303..39ba3e438 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -199,7 +199,7 @@ use std::str::FromStr; use std::sync::atomic::AtomicUsize; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex, OnceLock}; -use std::time::Duration; +use std::time::{Duration, Instant}; use time::{OffsetDateTime, format_description::well_known::Rfc3339}; use tokio::io::{AsyncRead, ReadBuf}; use tokio::sync::{OwnedSemaphorePermit, RwLock}; @@ -543,6 +543,9 @@ pub(super) fn map_quota_check_outcome(bucket: &str, outcome: Result S3Result<()> { + if result.uses_durable_reservations { + return Ok(()); + } let Some(quota_limit) = result.quota_limit else { return Ok(()); }; @@ -571,6 +574,25 @@ fn ensure_object_size_within_quota(result: &QuotaCheckResult, new_size: u64) -> Ok(()) } +fn ensure_legacy_archive_size_within_quota(result: &QuotaCheckResult, total_unpacked_size: u64) -> S3Result<()> { + if result.uses_durable_reservations { + return Ok(()); + } + let (Some(current_usage), Some(quota_limit)) = (result.current_usage, result.quota_limit) else { + return Ok(()); + }; + let expected_usage = current_usage + .checked_add(total_unpacked_size) + .ok_or_else(|| s3_error!(InvalidArgument, "Archive total size overflowed quota accounting"))?; + if expected_usage > quota_limit { + return Err(S3Error::with_message( + S3ErrorCode::InvalidRequest, + format!("Bucket quota exceeded. Current usage: {current_usage} bytes, limit: {quota_limit} bytes"), + )); + } + Ok(()) +} + fn quota_accounting_object_size(info: &ObjectInfo, fail_closed: bool) -> S3Result { match quota_object_size(info) { Ok(size) => Ok(size), @@ -5630,7 +5652,13 @@ impl DefaultObjectUsecase { // The app check preserves the existing S3 error contract; the storage // commit path reserves the exact net logical growth under its locks. - let quota_check = self.check_bucket_quota(&bucket, quota_operation, 0).await?; + let quota_check = self + .check_bucket_quota( + &bucket, + quota_operation, + u64::try_from(size).map_err(|_| S3Error::new(S3ErrorCode::UnexpectedContent))?, + ) + .await?; let quota_enabled = quota_check.as_ref().is_some_and(|result| result.quota_limit.is_some()); if quota_enabled && ciphertext_passthrough { return Err(S3Error::with_message( @@ -5639,7 +5667,8 @@ impl DefaultObjectUsecase { )); } - let ingress_stage_start = std::time::Instant::now(); + let put_stage_metrics_enabled = rustfs_io_metrics::put_stage_metrics_enabled(); + let ingress_stage_start = put_stage_metrics_enabled.then(Instant::now); let should_compress = is_disk_compressible(&req.headers, &key) && size > MIN_DISK_COMPRESSIBLE_SIZE as i64 && !ciphertext_passthrough; let server_side_encryption_requested = @@ -5703,9 +5732,13 @@ impl DefaultObjectUsecase { let Some(store) = self.object_store() else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; + let bucket_validate_stage_start = put_stage_metrics_enabled.then(Instant::now); validate_bucket_exists(&store, &bucket).await?; + rustfs_io_metrics::record_put_object_stage_duration_from("app_bucket_validate", bucket_validate_stage_start); + let sse_config_stage_start = put_stage_metrics_enabled.then(Instant::now); let bucket_sse_config = metadata_sys::get_sse_config(&bucket).await.ok(); + rustfs_io_metrics::record_put_object_stage_duration_from("app_sse_config_lookup", sse_config_stage_start); debug!( target: "rustfs::app::object_usecase", component = "app", @@ -5771,7 +5804,9 @@ impl DefaultObjectUsecase { let mut metadata = metadata.unwrap_or_default(); let has_explicit_object_lock_retention = object_lock_mode.is_some() || object_lock_retain_until_date.is_some(); + let object_lock_config_stage_start = put_stage_metrics_enabled.then(Instant::now); let object_lock_config_state = load_bucket_object_lock_config_state(&bucket).await?; + rustfs_io_metrics::record_put_object_stage_duration_from("app_object_lock_config_lookup", object_lock_config_stage_start); apply_put_request_metadata( &mut metadata, &req.headers, @@ -5793,6 +5828,7 @@ impl DefaultObjectUsecase { has_explicit_object_lock_retention, )?; + let put_opts_stage_start = put_stage_metrics_enabled.then(Instant::now); let mut opts: ObjectOptions = put_opts_with_replication_authorization( &bucket, &key, @@ -5806,6 +5842,7 @@ impl DefaultObjectUsecase { if let Some(quota_check) = quota_check.as_ref() { apply_quota_admission(&mut opts, quota_check)?; } + rustfs_io_metrics::record_put_object_stage_duration_from("app_put_opts_build", put_opts_stage_start); apply_bucket_generation_guard(&req, &bucket, &mut opts)?; apply_put_request_object_lock_opts( &bucket, @@ -5824,10 +5861,11 @@ impl DefaultObjectUsecase { // replication), the lookup is skipped and accounting is backfilled from // the dst xl.meta that rename_data already reads, saving a full-disk // metadata fanout per PUT. - let prelookup_required = quota_enabled || version_id.is_some() || object_lock_checks_required(&bucket).await; + let prelookup_required = version_id.is_some() || object_lock_checks_required_for_state(&object_lock_config_state); // Outer None = prelookup skipped (accounting comes from the commit // backfill); Some(inner) = the previous current size as observed by the // lookup, with the pre-#1009 semantics kept bit-for-bit. + let prelookup_stage_start = (prelookup_required && put_stage_metrics_enabled).then(Instant::now); let prelookup_previous_current_size: Option> = if prelookup_required { let current_opts: ObjectOptions = internal_object_info_lookup_opts( get_opts(&bucket, &key, version_id.clone(), None, &req.headers) @@ -5857,6 +5895,7 @@ impl DefaultObjectUsecase { } else { None }; + rustfs_io_metrics::record_put_object_stage_duration_from("app_prelookup", prelookup_stage_start); let actual_size = size; if !ciphertext_passthrough && let Some(quota_check) = quota_check.as_ref() { @@ -5951,15 +5990,13 @@ impl DefaultObjectUsecase { put_extra_checksum_headers = additional_checksum_echo_pairs(&opts.want_checksum); } rustfs_io_metrics::record_put_object_path(put_path); - rustfs_io_metrics::record_put_object_stage_duration( - "ingress_prepare", - ingress_stage_start.elapsed().as_secs_f64() * 1000.0, - ); + rustfs_io_metrics::record_put_object_stage_duration_from("ingress_prepare", ingress_stage_start); let mut helper = OperationHelper::new(&req, event_name, S3Operation::PutObject); let ssekms_context = extract_ssekms_context_from_headers(&req.headers)?; // Apply encryption using unified SSE API. + let encryption_stage_start = put_stage_metrics_enabled.then(Instant::now); let write_principal = SseKmsPrincipal::from_request(&req); let encryption_request = EncryptionRequest { bucket: &bucket, @@ -6003,6 +6040,7 @@ impl DefaultObjectUsecase { } reader = write_plan.apply(reader, actual_size).map_err(ApiError::from)?; + rustfs_io_metrics::record_put_object_stage_duration_from("app_encryption_prepare", encryption_stage_start); let mut reader = PutObjReader::new(reader); @@ -6019,9 +6057,11 @@ impl DefaultObjectUsecase { // post-commit schedule (see the reuse site further down), so a // replication-config hot update can no longer split the two phases // (https://github.com/rustfs/backlog/issues/1320). + let replication_decision_stage_start = put_stage_metrics_enabled.then(Instant::now); let dsc = must_replicate_object(&bucket, &key, &mt2, "".to_string(), opts.delete_marker_replication_status(), opts.clone()) .await; + rustfs_io_metrics::record_put_object_stage_duration_from("app_replication_decision", replication_decision_stage_start); if dsc.replicate_any() { insert_str(&mut opts.user_defined, SUFFIX_REPLICATION_TIMESTAMP, jiff::Zoned::now().to_string()); @@ -6033,7 +6073,12 @@ impl DefaultObjectUsecase { } let cache_adapter = self.object_data_cache(); + let cache_invalidate_before_stage_start = put_stage_metrics_enabled.then(Instant::now); let _ = invalidate_object_data_cache_before_mutation(&cache_adapter, &bucket, &key).await; + rustfs_io_metrics::record_put_object_stage_duration_from( + "app_cache_invalidate_before", + cache_invalidate_before_stage_start, + ); let store_put_watchdog = tokio_util::sync::CancellationToken::new(); spawn_traced({ @@ -6073,6 +6118,7 @@ impl DefaultObjectUsecase { let object_traffic_progress = object_traffic_health .as_deref() .and_then(ObjectTrafficHealth::track_write_storage); + let store_put_stage_start = put_stage_metrics_enabled.then(Instant::now); let (obj_info, backfilled_old_current_size) = match store .put_object_with_old_current_size(&bucket, &key, &mut reader, &opts) .await @@ -6098,6 +6144,7 @@ impl DefaultObjectUsecase { } Err(err) => { store_put_watchdog.cancel(); + rustfs_io_metrics::record_put_object_stage_duration_from("app_store_put", store_put_stage_start); warn!( target: "rustfs::app::object_usecase", event = EVENT_PUT_OBJECT_STORE_RETURNED, @@ -6119,10 +6166,12 @@ impl DefaultObjectUsecase { return result; } }; + rustfs_io_metrics::record_put_object_stage_duration_from("app_store_put", store_put_stage_start); drop(object_traffic_progress); #[cfg(test)] wait_for_put_post_store_test_hook(&bucket).await; + let post_store_stage_start = put_stage_metrics_enabled.then(Instant::now); maybe_enqueue_transition_immediate(&obj_info, LcEventSrc::S3PutObject).await; let _ = invalidate_object_data_cache_after_put_success(&cache_adapter, &bucket, &key).await; @@ -6222,10 +6271,13 @@ impl DefaultObjectUsecase { let result = Ok(response); let _ = helper.complete(&result); rustfs_scanner::record_dirty_usage_bucket(&bucket); + rustfs_io_metrics::record_put_object_stage_duration_from("app_post_store_bookkeeping", post_store_stage_start); // Record write operation for capacity management (inline to avoid per-request tokio::spawn overhead) + let capacity_update_stage_start = put_stage_metrics_enabled.then(Instant::now); let manager = get_capacity_manager(); manager.record_write_operation().await; + rustfs_io_metrics::record_put_object_stage_duration_from("app_capacity_update", capacity_update_stage_start); // Record PutObject metrics via zero-copy-metrics { @@ -7558,7 +7610,13 @@ impl DefaultObjectUsecase { src_info.user_defined = Arc::new(user_defined); - let quota_check = self.check_bucket_quota(&bucket, QuotaOperation::CopyObject, 0).await?; + let quota_check = self + .check_bucket_quota( + &bucket, + QuotaOperation::CopyObject, + u64::try_from(actual_size).map_err(|_| S3Error::new(S3ErrorCode::UnexpectedContent))?, + ) + .await?; let quota_enabled = quota_check.as_ref().is_some_and(|result| result.quota_limit.is_some()); if let Some(quota_check) = quota_check.as_ref() { apply_quota_admission(&mut dst_opts, quota_check)?; @@ -9190,7 +9248,13 @@ impl DefaultObjectUsecase { } validate_object_key(&key, "PUT")?; validate_table_catalog_object_mutation(&bucket, &key).await?; - let _ = self.check_bucket_quota(&bucket, QuotaOperation::PutObject, 0).await?; + let _ = self + .check_bucket_quota( + &bucket, + QuotaOperation::PutObject, + u64::try_from(size).map_err(|_| S3Error::new(S3ErrorCode::UnexpectedContent))?, + ) + .await?; // Apply adaptive buffer sizing based on file size for optimal streaming performance. // Uses workload profile configuration (enabled by default) to select appropriate buffer size. @@ -9336,6 +9400,9 @@ impl DefaultObjectUsecase { .checked_add(entry_size) .ok_or_else(|| s3_error!(InvalidArgument, "Archive total unpacked size overflowed while processing entries"))?; validate_put_object_extract_total_size(total_unpacked_size, extract_limits)?; + if let Some(quota_check) = extract_quota_check.as_ref() { + ensure_legacy_archive_size_within_quota(quota_check, total_unpacked_size)?; + } let mut size = i64::try_from(entry_size).map_err(|_| s3_error!(InvalidArgument, "Archive entry size does not fit into i64"))?; // mtime 0 means "unset" in tar headers, and xl.meta cannot represent an @@ -9644,6 +9711,13 @@ pub(super) async fn object_lock_checks_required(bucket: &str) -> bool { .map_or(true, |metadata| metadata.object_locking()) } +fn object_lock_checks_required_for_state(state: &metadata_sys::ObjectLockConfigState) -> bool { + match state { + metadata_sys::ObjectLockConfigState::Configured { .. } | metadata_sys::ObjectLockConfigState::Fabricated => true, + metadata_sys::ObjectLockConfigState::ConfirmedAbsent => false, + } +} + /// rustfs/backlog#1009: map the rename_data old-size backfill onto the /// `previous_current_size` value the usage-accounting helpers expect. Outer /// `None` = unknown (no quorum agreement, or a peer predates the field) — the @@ -10270,6 +10344,23 @@ mod tests { assert_eq!(err.message(), Some(ERR_OBJECT_LOCK_RETENTION_HEADERS_MUST_BE_PAIRED)); } + #[test] + fn object_lock_checks_required_reuses_authoritative_state() { + assert!(!object_lock_checks_required_for_state( + &metadata_sys::ObjectLockConfigState::ConfirmedAbsent + )); + + let configured = metadata_sys::ObjectLockConfigState::Configured { + config: ObjectLockConfiguration { + object_lock_enabled: Some(ObjectLockEnabled::from_static(ObjectLockEnabled::ENABLED)), + rule: None, + }, + updated_at: OffsetDateTime::now_utc(), + }; + assert!(object_lock_checks_required_for_state(&configured)); + assert!(object_lock_checks_required_for_state(&metadata_sys::ObjectLockConfigState::Fabricated)); + } + #[test] fn build_put_like_object_lock_metadata_rejects_retain_until_date_without_mode() { let retain_until = Timestamp::from(OffsetDateTime::now_utc().add(time::Duration::days(1))); @@ -17405,6 +17496,7 @@ mod tests { quota_limit: Some(2048), operation_size: 512, remaining: Some(512), + uses_durable_reservations: true, } } @@ -17446,6 +17538,57 @@ mod tests { assert!(!body_polled.load(Ordering::Acquire), "rejected ciphertext body must not be consumed"); } + #[tokio::test] + #[serial_test::serial] + async fn legacy_quota_rejects_full_put_before_polling_the_body() { + use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions}; + use std::sync::atomic::{AtomicBool, Ordering}; + + const GI_B: u64 = 1024 * 1024 * 1024; + let store = crate::app::gating_test_env::shared_gating_ecstore().await; + crate::app::runtime_sources::install_test_app_context(Arc::clone(&store)).await; + let bucket = format!("legacy-quota-{}", Uuid::new_v4().simple()); + store + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("create legacy quota test bucket"); + crate::app::storage_api::test::data_usage::seed_bucket_usage_memory_for_test(&bucket, 4 * GI_B).await; + let metadata_sys = DefaultObjectUsecase::from_global() + .bucket_metadata_sys() + .expect("test app context should expose bucket metadata"); + QuotaChecker::new(metadata_sys) + .set_quota_config( + &bucket, + BucketQuota { + quota: Some(5 * GI_B), + ..Default::default() + }, + ) + .await + .expect("configure legacy quota"); + + let body_polled = Arc::new(AtomicBool::new(false)); + let body_polled_in_stream = Arc::clone(&body_polled); + let body = StreamingBlob::wrap(futures::stream::once(async move { + body_polled_in_stream.store(true, Ordering::Release); + Ok::(Bytes::new()) + })); + let input = PutObjectInput::builder() + .bucket(bucket) + .key("object".to_string()) + .body(Some(body)) + .content_length(Some(i64::try_from(2 * GI_B).expect("test size should fit i64"))) + .build() + .expect("legacy quota PUT input should build"); + + let err = DefaultObjectUsecase::from_global() + .execute_put_object(&FS::new(), build_request(input, Method::PUT)) + .await + .expect_err("4 GiB used plus a 2 GiB PUT must exceed a 5 GiB legacy quota"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + assert!(!body_polled.load(Ordering::Acquire), "legacy quota rejection must not consume the body"); + } + #[test] fn quota_admission_allows_within_limit() { let result = map_quota_check_outcome("bucket", Ok(quota_result(true))).expect("an allowed result admits the write"); @@ -17529,6 +17672,45 @@ mod tests { .expect("second within-limit PUT should commit"); } + #[tokio::test] + #[serial_test::serial] + async fn put_rejects_rotated_quota_capability_before_rename() { + use crate::app::storage_api::test::set_disk::{PutObjectCommitBarrier, PutObjectCommitPause}; + + let (store, bucket) = crate::app::gating_test_env::durable_quota_test_bucket("rotated-proof-put-quota", 4096).await; + let barrier = PutObjectCommitBarrier::install(&bucket, "object", PutObjectCommitPause::BeforeQuotaRename); + let put_store = Arc::clone(&store); + let put_bucket = bucket.clone(); + let put = tokio::spawn(async move { + let mut reader = PutObjReader::from_vec(vec![0x77; 4096]); + put_store + .put_object(&put_bucket, "object", &mut reader, &ObjectOptions::default()) + .await + }); + barrier.wait_until_paused().await; + assert!( + crate::storage::storage_api::ecstore_notification::rotate_cross_pool_fence_fleet_proof_for_test(), + "the gating environment must have a current fleet proof" + ); + barrier.release(); + + let err = put + .await + .expect("PUT task should not panic") + .expect_err("a replaced fleet proof must fence the authoritative rename"); + assert!(matches!( + err, + StorageError::NamespaceLockQuorumUnavailable { + mode: "quota_reservation", + .. + } + )); + store + .get_object_info(&bucket, "object", &ObjectOptions::default()) + .await + .expect_err("proof rotation before rename must leave no committed object"); + } + #[tokio::test] #[serial_test::serial] async fn durable_quota_reclaims_overwrites_and_deleted_bytes() { @@ -17566,6 +17748,26 @@ mod tests { )); } + #[tokio::test] + #[serial_test::serial] + async fn data_movement_put_has_zero_quota_growth() { + let (store, bucket) = crate::app::gating_test_env::durable_quota_test_bucket("data-movement-put-quota", 0).await; + let mut reader = PutObjReader::from_vec(vec![0x79; 4096]); + let stored = store + .put_object( + &bucket, + "object", + &mut reader, + &ObjectOptions { + data_movement: true, + ..Default::default() + }, + ) + .await + .expect("moving an already-accounted object between pools must have zero quota growth"); + assert_eq!(stored.size, 4096); + } + #[tokio::test] #[serial_test::serial] async fn cancelled_put_releases_durable_quota_reservation() { @@ -17809,6 +18011,27 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::ServiceUnavailable); } + #[test] + fn legacy_archive_quota_rejects_cumulative_size_and_overflow() { + let legacy = QuotaCheckResult { + allowed: true, + current_usage: Some(4), + quota_limit: Some(5), + operation_size: 0, + remaining: Some(1), + uses_durable_reservations: false, + }; + assert!(ensure_legacy_archive_size_within_quota(&legacy, 2).is_err()); + assert!(ensure_legacy_archive_size_within_quota(&legacy, 1).is_ok()); + + let maxed = QuotaCheckResult { + current_usage: Some(u64::MAX), + quota_limit: Some(u64::MAX), + ..legacy + }; + assert!(ensure_legacy_archive_size_within_quota(&maxed, 1).is_err()); + } + #[test] fn early_quota_filter_rejects_only_an_individually_impossible_object() { let stale_full_usage = QuotaCheckResult { @@ -17817,6 +18040,7 @@ mod tests { quota_limit: Some(4096), operation_size: 0, remaining: Some(0), + uses_durable_reservations: true, }; ensure_object_size_within_quota(&stale_full_usage, 4096) diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index e0bef7005..05183969c 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -1216,7 +1216,8 @@ pub(crate) mod test { }; pub(crate) mod set_disk { pub(crate) use crate::storage::storage_api::ecstore_set_disk::{ - PutObjectCommitBarrier, PutObjectCommitPause, fail_next_quota_ledger_save_for_test, + MultipartCommitBarrier, MultipartCommitPause, PutObjectCommitBarrier, PutObjectCommitPause, + fail_next_quota_ledger_save_for_test, }; } diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs index 4eb6621c6..49ac36319 100644 --- a/rustfs/src/error.rs +++ b/rustfs/src/error.rs @@ -398,7 +398,6 @@ impl From for ApiError { QuotaError::ConfigNotFound { .. } => S3ErrorCode::NoSuchBucket, QuotaError::UsageUnavailable { .. } => S3ErrorCode::ServiceUnavailable, QuotaError::InvalidConfig { .. } => S3ErrorCode::InvalidArgument, - QuotaError::StorageError(StorageError::NamespaceLockQuorumUnavailable { .. }) => S3ErrorCode::ServiceUnavailable, QuotaError::StorageError(_) => S3ErrorCode::InternalError, }; @@ -552,20 +551,6 @@ mod tests { assert_eq!(api_error.message, "The service is unavailable. Please retry."); } - #[test] - fn stale_quota_capability_maps_to_retryable_error() { - let api_error = ApiError::from(QuotaError::StorageError(StorageError::NamespaceLockQuorumUnavailable { - mode: "quota_capability", - bucket: "bucket".to_string(), - object: rustfs_config::QUOTA_CONFIG_FILE.to_string(), - required: 1, - achieved: 0, - })); - - assert_eq!(api_error.code, S3ErrorCode::ServiceUnavailable); - assert_eq!(api_error.message, "The service is unavailable. Please retry."); - } - #[test] fn test_kms_cryptographic_error_is_not_retryable() { let api_error = ApiError::from(StorageError::other(rustfs_kms::KmsError::cryptographic_error( diff --git a/rustfs/src/startup_observability.rs b/rustfs/src/startup_observability.rs index 0dd66b875..01ec050b8 100644 --- a/rustfs/src/startup_observability.rs +++ b/rustfs/src/startup_observability.rs @@ -24,14 +24,63 @@ pub(crate) async fn init_observability_runtime(store: Arc, ctx: Cancell init_update_check(); crate::allocator_reclaim::init_allocator_reclaim(ctx.clone()); - if startup_runtime_sources::observability_metric_enabled() { + let metrics_enabled = startup_runtime_sources::observability_metric_enabled(); + configure_metric_gates(metrics_enabled); + + if metrics_enabled { // Load persisted compression stats into memory early, before any PUTs can occur. init_compression_total_memory_from_backend(store).await; - startup_runtime_sources::set_put_stage_metrics_enabled(true); - startup_runtime_sources::set_get_stage_metrics_enabled(true); - startup_runtime_sources::set_metrics_enabled(true); startup_runtime_sources::init_metrics_runtime(ctx.clone()); crate::memory_observability::init_memory_observability(ctx.clone()); init_auto_tuner(ctx).await; } } + +fn configure_metric_gates(metrics_enabled: bool) { + let put_stage_metrics_enabled = metrics_enabled + && rustfs_utils::get_env_bool( + rustfs_config::observability::ENV_OBS_PUT_STAGE_METRICS_ENABLED, + rustfs_config::DEFAULT_OBS_PUT_STAGE_METRICS_ENABLED, + ); + startup_runtime_sources::set_put_stage_metrics_enabled(put_stage_metrics_enabled); + startup_runtime_sources::set_get_stage_metrics_enabled(metrics_enabled); + startup_runtime_sources::set_metrics_enabled(metrics_enabled); +} + +#[cfg(test)] +mod tests { + use super::*; + + const PUT_STAGE_ENV: &str = rustfs_config::observability::ENV_OBS_PUT_STAGE_METRICS_ENABLED; + + #[test] + #[serial_test::serial] + fn put_stage_metrics_require_explicit_opt_in() { + let previous_metrics = rustfs_io_metrics::metrics_enabled(); + let previous_get_stages = rustfs_io_metrics::get_stage_metrics_enabled(); + let previous_put_stages = rustfs_io_metrics::put_stage_metrics_enabled(); + + temp_env::with_var(PUT_STAGE_ENV, None::<&str>, || { + configure_metric_gates(true); + assert!(rustfs_io_metrics::metrics_enabled()); + assert!(rustfs_io_metrics::get_stage_metrics_enabled()); + assert!(!rustfs_io_metrics::put_stage_metrics_enabled()); + }); + + temp_env::with_var(PUT_STAGE_ENV, Some("true"), || { + configure_metric_gates(true); + assert!(rustfs_io_metrics::metrics_enabled()); + assert!(rustfs_io_metrics::get_stage_metrics_enabled()); + assert!(rustfs_io_metrics::put_stage_metrics_enabled()); + + configure_metric_gates(false); + assert!(!rustfs_io_metrics::metrics_enabled()); + assert!(!rustfs_io_metrics::get_stage_metrics_enabled()); + assert!(!rustfs_io_metrics::put_stage_metrics_enabled()); + }); + + startup_runtime_sources::set_metrics_enabled(previous_metrics); + startup_runtime_sources::set_get_stage_metrics_enabled(previous_get_stages); + startup_runtime_sources::set_put_stage_metrics_enabled(previous_put_stages); + } +} diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index 9043908d2..131f3ce1f 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -153,7 +153,9 @@ fn remove_heal_control_replay( static HEAL_CONTROL_REPLAY_CACHE: OnceLock>>> = OnceLock::new(); static NODE_CAPABILITY_SERVER_EPOCH: LazyLock = LazyLock::new(Uuid::new_v4); -const CROSS_POOL_FENCE_SUPPORTED_VERSION: u32 = 1; +// RUSTFS_COMPAT_TODO(cross-pool-fence-v1): advertise unsupported during predeployment. Remove after composite acquisition, +// activation fencing, fleet proof, commit-time proof revalidation, and fail-closed revocation ship together. +const CROSS_POOL_FENCE_SUPPORTED_VERSION: u32 = 0; fn admit_heal_control_replay( replay_cache: &mut HashMap>, @@ -2205,7 +2207,7 @@ mod tests { use crate::storage::storage_api::rpc_consumer::node_service::{DiskError, HealBucketInfo, HealEndpoint}; use crate::storage::storage_api::set_tonic_canonical_body_digest; use crate::storage::storage_api::{ - Endpoint, RUSTFS_META_BUCKET, SnapshotLeaseToken, + Endpoint, ecstore_layout::{EndpointServerPools, Endpoints, PoolEndpoints}, }; use bytes::Bytes; @@ -2943,22 +2945,14 @@ mod tests { } #[tokio::test] - #[serial_test::serial] - async fn snapshot_lease_handlers_forward_to_local_disk() { - let temp_dir = tempfile::tempdir().expect("snapshot lease RPC test directory"); - let env = rustfs_test_utils::TestECStoreEnv::builder() - .base_dir(temp_dir.path()) - .init_bucket_metadata(false) - .build() - .await; + async fn snapshot_lease_acquire_and_renew_handlers_fail_closed() { let service = make_server(); - let disk = env.disk_paths[0].to_string_lossy().into_owned(); - let fence_path = format!("tmp/quota-mutation-fences/{}", "0".repeat(64)); + let disk = "http://node-a:9000/data/rustfs0".to_string(); let mut acquire = Request::new(SnapshotLeaseRequest { disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), + volume: "v".into(), + path: "p".into(), ttl_ms: 60_000, }); let acquire_body = @@ -2968,18 +2962,14 @@ mod tests { let acquire = service .acquire_snapshot_lease(acquire) .await - .expect("acquire should return a protocol response") + .expect("disabled acquire should return a protocol response") .into_inner(); - assert!(acquire.success, "local disk should acquire the mutation fence"); - assert_eq!(acquire.protocol_version, 1); - assert!(acquire.error.is_none()); - let token = SnapshotLeaseToken::from_slice(&acquire.token).expect("acquire should return a valid token"); let mut renew = Request::new(SnapshotLeaseRenewRequest { - disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), - token: token.as_bytes().to_vec().into(), + disk, + volume: "v".into(), + path: "p".into(), + token: vec![1; 16].into(), ttl_ms: 60_000, }); let renew_body = rustfs_protos::canonical_snapshot_lease_renew_request_body(renew.get_ref()) @@ -2989,103 +2979,15 @@ mod tests { let renew = service .renew_snapshot_lease(renew) .await - .expect("renew should return a protocol response") + .expect("disabled renew should return a protocol response") .into_inner(); - assert!(renew.success, "local disk should renew the mutation fence"); - assert_eq!(renew.protocol_version, 1); - assert!(renew.error.is_none()); - let renewed = SnapshotLeaseToken::from_slice(&renew.token).expect("renew should return a valid token"); - assert_ne!(renewed, token); - let mut release = Request::new(SnapshotLeaseReleaseRequest { - disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), - token: renewed.as_bytes().to_vec().into(), - }); - let release_body = rustfs_protos::canonical_snapshot_lease_release_request_body(release.get_ref()) - .expect("release request body should encode"); - set_tonic_canonical_body_digest(&mut release, &release_body).expect("release digest metadata should encode"); - mark_v2_authenticated(&mut release); - let release = service - .release_snapshot_lease(release) - .await - .expect("release should return a protocol response") - .into_inner(); - assert!(release.success, "local disk should release the renewed token"); - assert!(release.error.is_none()); - - let mut acquire = Request::new(SnapshotLeaseRequest { - disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), - ttl_ms: 60_000, - }); - let acquire_body = - rustfs_protos::canonical_snapshot_lease_request_body(acquire.get_ref()).expect("acquire request body should encode"); - set_tonic_canonical_body_digest(&mut acquire, &acquire_body).expect("acquire digest metadata should encode"); - mark_v2_authenticated(&mut acquire); - let active_token = service - .acquire_snapshot_lease(acquire) - .await - .expect("second acquire should return a protocol response") - .into_inner(); - assert!(active_token.success); - - let mut revoke = Request::new(SnapshotLeaseReleaseRequest { - disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), - token: SnapshotLeaseToken::revoke_all().as_bytes().to_vec().into(), - }); - let revoke_body = rustfs_protos::canonical_snapshot_lease_release_request_body(revoke.get_ref()) - .expect("revoke-all request body should encode"); - set_tonic_canonical_body_digest(&mut revoke, &revoke_body).expect("revoke-all digest metadata should encode"); - mark_v2_authenticated(&mut revoke); - let revoke = service - .release_snapshot_lease(revoke) - .await - .expect("revoke-all should return a protocol response") - .into_inner(); - assert!(revoke.success, "nil revoke-all sentinel must reach the local disk"); - assert!(revoke.error.is_none()); - - let mut stale_renew = Request::new(SnapshotLeaseRenewRequest { - disk: disk.clone(), - volume: RUSTFS_META_BUCKET.into(), - path: fence_path.clone(), - token: active_token.token, - ttl_ms: 60_000, - }); - let stale_renew_body = rustfs_protos::canonical_snapshot_lease_renew_request_body(stale_renew.get_ref()) - .expect("stale renew request body should encode"); - set_tonic_canonical_body_digest(&mut stale_renew, &stale_renew_body).expect("stale renew digest metadata should encode"); - mark_v2_authenticated(&mut stale_renew); - let stale_renew = service - .renew_snapshot_lease(stale_renew) - .await - .expect("stale renew should return a protocol response") - .into_inner(); - assert!(!stale_renew.success, "revoke-all must invalidate active tokens"); - assert!(stale_renew.token.is_empty()); - assert!(stale_renew.error.is_some()); - - let mut malformed = Request::new(SnapshotLeaseRenewRequest { - disk, - volume: RUSTFS_META_BUCKET.into(), - path: fence_path, - token: vec![1; 15].into(), - ttl_ms: 60_000, - }); - let malformed_body = rustfs_protos::canonical_snapshot_lease_renew_request_body(malformed.get_ref()) - .expect("malformed renew request body should encode"); - set_tonic_canonical_body_digest(&mut malformed, &malformed_body).expect("malformed renew digest metadata should encode"); - mark_v2_authenticated(&mut malformed); - let malformed = service - .renew_snapshot_lease(malformed) - .await - .expect_err("a malformed non-nil token must be rejected"); - assert_eq!(malformed.code(), tonic::Code::InvalidArgument); + for response in [acquire, renew] { + assert!(!response.success); + assert!(response.token.is_empty()); + assert_eq!(response.protocol_version, 1); + assert_eq!(response.error, Some(DiskError::UnsupportedDisk.into())); + } } #[tokio::test] @@ -3440,7 +3342,7 @@ mod tests { } #[tokio::test] - async fn cross_pool_fence_probe_authenticates_supported_v1_state() { + async fn cross_pool_fence_probe_authenticates_unsupported_rollout_state() { let _ = rustfs_credentials::set_global_rpc_secret("cross-pool-fence-node-service-test-secret".to_string()); let endpoints = heal_control_test_endpoints_with_coordinator("node-0", true); assert!( @@ -3505,7 +3407,7 @@ mod tests { assert!(response.success); assert_eq!(response.error_info, None); - assert_eq!(&response.result[..4], &super::CROSS_POOL_FENCE_SUPPORTED_VERSION.to_be_bytes()); + assert_eq!(&response.result[..4], &0_u32.to_be_bytes()); let (topology_member, process_epoch) = rustfs_protos::decode_remote_version_state_capability(&response.result[4..]) .expect("capability identity should decode"); assert_eq!(topology_member, "node-a:9000"); diff --git a/rustfs/src/storage/rpc/node_service/disk.rs b/rustfs/src/storage/rpc/node_service/disk.rs index d1dc1c8a1..1dc45beba 100644 --- a/rustfs/src/storage/rpc/node_service/disk.rs +++ b/rustfs/src/storage/rpc/node_service/disk.rs @@ -1521,8 +1521,8 @@ mod tests { encode_read_multiple_response_payloads, encode_rename_data_response_payloads, }; use crate::storage::storage_api::ReadMultipleResp; + use crate::storage::storage_api::RenameDataResp; use crate::storage::storage_api::rpc_consumer::node_service::BatchReadVersionResp; - use crate::storage::storage_api::{DiskError, RenameDataResp}; use rustfs_filemeta::FileInfo; use rustfs_io_metrics::internode_metrics::global_internode_metrics; use serde::{Deserialize, Serialize}; diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index d7453f5c5..568ec0cf4 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -485,6 +485,8 @@ pub(crate) mod ecstore_metrics { #[allow(unused_imports)] pub(crate) mod ecstore_notification { + #[cfg(test)] + pub(crate) use rustfs_ecstore::api::notification::rotate_cross_pool_fence_fleet_proof_for_test; pub(crate) use rustfs_ecstore::api::notification::{ CrossPoolFenceFleetProofToken, NotificationSys, acquire_cross_pool_fence_fleet_proof, cross_pool_fence_fleet_proof_matches, get_global_notification_sys, new_global_notification_sys, @@ -548,7 +550,8 @@ pub(crate) mod ecstore_test_support { pub(crate) mod ecstore_set_disk { #[cfg(test)] pub(crate) use rustfs_ecstore::api::set_disk::test_util::{ - PutObjectCommitBarrier, PutObjectCommitPause, fail_next_quota_ledger_save_for_test, + MultipartCommitBarrier, MultipartCommitPause, PutObjectCommitBarrier, PutObjectCommitPause, + fail_next_quota_ledger_save_for_test, }; pub(crate) use rustfs_ecstore::api::set_disk::{ DEFAULT_READ_BUFFER_SIZE, file_info_quorum_hash, get_lock_acquire_timeout, is_valid_storage_class, diff --git a/scripts/check_s3s_footprint.sh b/scripts/check_s3s_footprint.sh old mode 100755 new mode 100644