From 04e1ea227afc40fad583acceb0ef7df2c5ceea5a Mon Sep 17 00:00:00 2001 From: cxymds Date: Sat, 22 Aug 2026 19:11:48 +0800 Subject: [PATCH 1/8] fix(scanner): isolate corrupt cycle state (#6354) * fix(scanner): isolate corrupt cycle state * fix(scanner): preserve newer state during recovery reset * fix(scanner): fence recovery reset state * fix(scanner): reject terminal recovery epochs * fix(scanner): reject trailing cycle state bytes * fix(scanner): reject terminal leadership epochs * fix(scanner): retain recovery wake notifications * fix(scanner): recover from oversized markers --- crates/scanner/src/data_usage_define.rs | 33 + .../src/data_usage_define/persistence.rs | 29 +- crates/scanner/src/lib.rs | 5 +- crates/scanner/src/scanner.rs | 127 +- crates/scanner/src/scanner/cycle_state.rs | 1123 ++++++++++++++++- crates/scanner/src/scanner/leadership.rs | 2 +- crates/scanner/src/scanner/tests.rs | 931 +++++++++++++- rustfs/src/admin/handlers/mod.rs | 1 + rustfs/src/admin/handlers/scanner.rs | 97 +- rustfs/src/admin/route_policy.rs | 12 + rustfs/src/admin/route_registration_test.rs | 3 + 11 files changed, 2264 insertions(+), 99 deletions(-) diff --git a/crates/scanner/src/data_usage_define.rs b/crates/scanner/src/data_usage_define.rs index 75c3ca06e..c6ecdd489 100644 --- a/crates/scanner/src/data_usage_define.rs +++ b/crates/scanner/src/data_usage_define.rs @@ -125,6 +125,34 @@ pub(crate) async fn read_config_with_revision( } } +/// Read only the object revision without materializing its body. +pub(crate) async fn read_config_revision(store: Arc, path: &str) -> StorageResult { + match store + .get_object_reader( + RUSTFS_META_BUCKET, + path, + None, + HeaderMap::new(), + &ObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(reader) => reader + .object_info + .etag + .filter(|etag| !etag.is_empty()) + .map(DataUsageCacheRevision::Etag) + .ok_or_else(|| StorageError::other(format!("scanner config object {path} has no ETag"))), + Err(Error::FileNotFound | Error::VolumeNotFound | Error::ObjectNotFound(_, _) | Error::BucketNotFound(_)) => { + Ok(DataUsageCacheRevision::Missing) + } + Err(err) => Err(err), + } +} + #[derive(Clone, Debug)] pub(crate) struct DataUsageCacheRevisions { main: DataUsageCacheRevision, @@ -146,6 +174,11 @@ pub static LEGACY_DATA_USAGE_OBJ_NAME_PATH: LazyLock = pub static DATA_USAGE_BLOOM_NAME_PATH: LazyLock = LazyLock::new(|| format!("{BUCKET_META_PREFIX}{SLASH_SEPARATOR}{DATA_USAGE_BLOOM_NAME}")); +/// Durable companion object for a cycle-state object which cannot be decoded. +/// The primary object is deliberately never replaced or deleted by recovery. +pub static DATA_USAGE_BLOOM_RECOVERY_PATH: LazyLock = + LazyLock::new(|| format!("{}.recovery-required.json", DATA_USAGE_BLOOM_NAME_PATH.as_str())); + pub static BACKGROUND_HEAL_INFO_PATH: LazyLock = LazyLock::new(|| format!("{BUCKET_META_PREFIX}{SLASH_SEPARATOR}.background-heal.json")); diff --git a/crates/scanner/src/data_usage_define/persistence.rs b/crates/scanner/src/data_usage_define/persistence.rs index 2ac453cf4..b0a28f504 100644 --- a/crates/scanner/src/data_usage_define/persistence.rs +++ b/crates/scanner/src/data_usage_define/persistence.rs @@ -74,7 +74,7 @@ impl DataUsageCache { let loaded = Self::load_cache(store.clone(), name).await?; let backup = match loaded.backup_revision { Some(revision) => Some(revision), - None => match Self::revision_for_path(store, &backup_path).await { + None => match read_config_revision(store, &backup_path).await { Ok(revision) => Some(revision), Err(err) => { counter!(METRIC_CACHE_BACKUP_REVISION_FAILURE_TOTAL).increment(1); @@ -336,33 +336,6 @@ impl DataUsageCache { } } - async fn revision_for_path(store: Arc, path: &str) -> StorageResult { - match store - .get_object_reader( - RUSTFS_META_BUCKET, - path, - None, - HeaderMap::new(), - &ObjectOptions { - no_lock: true, - ..Default::default() - }, - ) - .await - { - Ok(reader) => reader - .object_info - .etag - .filter(|etag| !etag.is_empty()) - .map(DataUsageCacheRevision::Etag) - .ok_or_else(|| StorageError::other(format!("scanner cache object {path} has no ETag"))), - Err(Error::FileNotFound | Error::VolumeNotFound | Error::ObjectNotFound(_, _) | Error::BucketNotFound(_)) => { - Ok(DataUsageCacheRevision::Missing) - } - Err(err) => Err(err), - } - } - pub(super) fn cache_save_timeout() -> Duration { crate::runtime_config::scanner_cache_save_timeout() } diff --git a/crates/scanner/src/lib.rs b/crates/scanner/src/lib.rs index a1e37f14e..ab01964a5 100644 --- a/crates/scanner/src/lib.rs +++ b/crates/scanner/src/lib.rs @@ -75,7 +75,10 @@ pub use remote_scanner::{ }; pub use runtime_config::{apply_scanner_runtime_config, scanner_runtime_config_status, validate_scanner_runtime_config}; pub use rustfs_common::last_minute; -pub use scanner::{ScannerCycleScheduleStatus, init_data_scanner, scanner_cycle_schedule_status, scanner_topology_digest}; +pub use scanner::{ + ScannerCycleRecoveryMarker, ScannerCycleRecoveryStatus, ScannerCycleScheduleStatus, init_data_scanner, + reset_scanner_cycle_recovery, scanner_cycle_recovery_status, scanner_cycle_schedule_status, scanner_topology_digest, +}; pub use scanner_io::{ ScannerDirtyUsageAckError, ScannerDirtyUsageState, acknowledge_dirty_usage_generation, clear_dirty_usage_bucket, record_dirty_usage_bucket, record_scanner_maintenance_change, scanner_activity_epoch, scanner_dirty_usage_state, diff --git a/crates/scanner/src/scanner.rs b/crates/scanner/src/scanner.rs index 4db558a54..7a4a482cf 100644 --- a/crates/scanner/src/scanner.rs +++ b/crates/scanner/src/scanner.rs @@ -20,7 +20,7 @@ use std::sync::{Arc, LazyLock, RwLock}; use crate::data_usage_define::{ BACKGROUND_HEAL_INFO_PATH, DATA_USAGE_BLOOM_NAME_PATH, DATA_USAGE_OBJ_NAME_PATH, DATA_USAGE_OBSERVED_OBJ_NAME_PATH, - DataUsageCache, DataUsageCacheRevision, LEGACY_DATA_USAGE_OBJ_NAME_PATH, read_config_with_revision, + DataUsageCache, DataUsageCacheRevision, LEGACY_DATA_USAGE_OBJ_NAME_PATH, read_config_revision, read_config_with_revision, }; use crate::runtime_config::{ ScannerRuntimeConfig, ScannerRuntimeConfigSource, refresh_scanner_runtime_config_from_global, scanner_bitrot_cycle, @@ -54,9 +54,7 @@ use rustfs_config::{ENV_SCANNER_CYCLE, ENV_SCANNER_SPEED, ENV_SCANNER_START_DELA use rustfs_data_usage::observed_data_usage_is_newer; use serde::{Deserialize, Serialize}; use sha2::{Digest as _, Sha256}; -#[cfg(test)] -use tokio::sync::Notify; -use tokio::sync::mpsc; +use tokio::sync::{Notify, mpsc}; use tokio::time::{Duration, Instant}; use tokio_util::sync::CancellationToken; use tokio_util::task::AbortOnDropHandle; @@ -104,6 +102,13 @@ const CLEAN_IDLE_BACKOFF_FACTOR: u32 = 2; /// unavailable peer cannot drive a tight retry loop. const SCANNER_RETRY_BASE_INTERVAL: Duration = Duration::from_secs(5); const SCANNER_RETRY_MAX_INTERVAL: Duration = Duration::from_secs(30 * 60); +/// A transient backend outage remains self-healing after the short retry +/// budget is exhausted, but the probe is intentionally sparse until storage +/// recovers or an operator reset wakes the scanner. +const SCANNER_CYCLE_RECOVERY_PAUSED_INTERVAL: Duration = Duration::from_secs(5 * 60); +/// Permanent recovery states still get a sparse status probe so a reset that +/// races the wait registration cannot leave the scanner asleep forever. +const SCANNER_CYCLE_RECOVERY_BLOCKED_PROBE_INTERVAL: Duration = Duration::from_secs(5 * 60); const SCANNER_LEADER_LOCK_POLL_INTERVAL: Duration = Duration::from_secs(1); #[cfg(not(test))] const SCANNER_LOCK_LOSS_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(30); @@ -125,6 +130,12 @@ type ScannerCycleStatePersistTestHook = (u64, Arc); static SCANNER_CYCLE_STATE_PERSIST_TEST_HOOK: LazyLock>> = LazyLock::new(|| StdMutex::new(None)); +static SCANNER_CYCLE_RECOVERY_WAKE: LazyLock = LazyLock::new(Notify::new); + +pub(super) fn notify_scanner_cycle_recovery_wake() { + SCANNER_CYCLE_RECOVERY_WAKE.notify_one(); +} + #[cfg(test)] struct ScannerCycleStatePersistTestHookGuard; @@ -576,19 +587,21 @@ pub async fn init_data_scanner(ctx: CancellationToken, storeapi: Arc) { tokio::time::sleep(sleep_time).await; } + let mut transient_backoff = ScannerRetryBackoff::default(); + let mut recovery_retry_count = 0_u32; loop { if ctx_clone.is_cancelled() { break; } - if let Err(e) = run_data_scanner_with_maintenance_state( + let run_result = run_data_scanner_with_maintenance_state( ctx_clone.clone(), storeapi_clone.clone(), startup_features, startup_maintenance_generation, ) - .await - { + .await; + if let Err(e) = &run_result { error!( target: "rustfs::scanner", event = EVENT_SCANNER_CYCLE_STATE, @@ -599,11 +612,52 @@ pub async fn init_data_scanner(ctx: CancellationToken, storeapi: Arc) { "Scanner runtime iteration failed" ); } + let recovery_status = scanner_cycle_recovery_status(); + if recovery_status.retryable { + recovery_retry_count = recovery_retry_count.saturating_add(1); + let _ = record_scanner_cycle_recovery_retry(recovery_retry_count); + } else { + recovery_retry_count = 0; + } + + let recovery_status = scanner_cycle_recovery_status(); + if recovery_status.state == "paused" { + transient_backoff.record_retryable_cycle(false); + tokio::select! { + _ = ctx_clone.cancelled() => break, + _ = SCANNER_CYCLE_RECOVERY_WAKE.notified() => {}, + _ = tokio::time::sleep(SCANNER_CYCLE_RECOVERY_PAUSED_INTERVAL) => {}, + } + recovery_retry_count = 0; + continue; + } + if !recovery_status.retryable + && matches!(recovery_status.state.as_str(), "blocked" | "recovery-required" | "cleanup-pending") + { + transient_backoff.record_retryable_cycle(false); + tokio::select! { + _ = ctx_clone.cancelled() => break, + _ = SCANNER_CYCLE_RECOVERY_WAKE.notified() => {}, + _ = tokio::time::sleep(SCANNER_CYCLE_RECOVERY_BLOCKED_PROBE_INTERVAL) => {}, + } + continue; + } + + let retry_delay = if recovery_status.retryable || run_result.is_err() { + transient_backoff.record_retryable_cycle(true); + transient_backoff + .retry_interval(scanner_cycle_interval()) + .unwrap_or(SCANNER_RETRY_BASE_INTERVAL) + } else { + transient_backoff.record_retryable_cycle(false); + randomized_cycle_delay() + }; // Backoff before retrying after lock contention or scanner-level failures. // Keep this cancellation-aware so shutdown is not delayed by backoff sleep. tokio::select! { _ = ctx_clone.cancelled() => break, - _ = tokio::time::sleep(randomized_cycle_delay()) => {} + _ = SCANNER_CYCLE_RECOVERY_WAKE.notified() => {}, + _ = tokio::time::sleep(retry_delay) => {} } } }); @@ -1606,40 +1660,22 @@ async fn run_data_scanner_with_maintenance_state( observe_scanner_activity(&storeapi, distributed, &mut scanner_activity_seen).await; } - let (buf, mut cycle_revision) = match read_config_with_revision(storeapi.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()).await { - Ok((buf, revision)) => (buf.unwrap_or_default(), revision), - Err(err) => { - error!( - target: "rustfs::scanner", - event = EVENT_SCANNER_PERSIST_STATE, - component = LOG_COMPONENT_SCANNER, - subsystem = LOG_SUBSYSTEM_RUNTIME, - path = %&*DATA_USAGE_BLOOM_NAME_PATH, - state = "revision_load_failed", - error = %err, - "Scanner cycle state revision load failed" - ); - global_metrics().set_cycle(None).await; - return Ok(()); - } - }; - let (mut cycle_info, mut leader_epoch) = match decode_scanner_cycle_state_for_startup(&buf) { - Ok(state) => state, - Err(err) => { - error!( - target: "rustfs::scanner", - event = EVENT_SCANNER_PERSIST_STATE, - component = LOG_COMPONENT_SCANNER, - subsystem = LOG_SUBSYSTEM_RUNTIME, - path = %&*DATA_USAGE_BLOOM_NAME_PATH, - state = "cycle_decode_failed", - error = %err, - "Scanner stopped because persisted cycle state is invalid" - ); - global_metrics().set_cycle(None).await; - return Ok(()); - } - }; + let (mut cycle_info, mut leader_epoch, mut cycle_revision) = + match load_scanner_cycle_state_for_startup(storeapi.clone()).await { + ScannerCycleStateStartup::Ready { + cycle, + leader_epoch, + revision, + } => (cycle, leader_epoch, revision), + ScannerCycleStateStartup::Blocked => { + global_metrics().set_cycle(None).await; + return Ok(()); + } + ScannerCycleStateStartup::Transient(err) => { + global_metrics().set_cycle(None).await; + return Err(err); + } + }; let usage_floor = match persisted_usage_floor(storeapi.clone()).await { Ok(floor) => floor, Err(err) => { @@ -2219,7 +2255,12 @@ pub(crate) use activity::{ pub(crate) use activity::{ScannerCycleOutcome, scanner_cycle_outcome_with_pending_maintenance}; #[cfg(test)] pub(crate) use cycle_state::encode_scanner_cycle_fence_for_test; -pub(crate) use cycle_state::{current_scanner_leader_epoch, decode_persisted_scanner_cycle_fence}; +pub use cycle_state::{ + ScannerCycleRecoveryMarker, ScannerCycleRecoveryStatus, reset_scanner_cycle_recovery, scanner_cycle_recovery_status, +}; +pub(crate) use cycle_state::{ + current_scanner_leader_epoch, decode_persisted_scanner_cycle_fence, load_scanner_cycle_state_for_startup, +}; pub use heal_info::{BackgroundHealInfo, read_background_heal_info, save_background_heal_info}; pub use usage_store::store_data_usage_in_backend; diff --git a/crates/scanner/src/scanner/cycle_state.rs b/crates/scanner/src/scanner/cycle_state.rs index 6f3af4b81..32c893fdf 100644 --- a/crates/scanner/src/scanner/cycle_state.rs +++ b/crates/scanner/src/scanner/cycle_state.rs @@ -13,6 +13,1067 @@ // limitations under the License. /// Scanner cycle-state codec, persisted usage floors, and cycle-state persistence. use super::*; +use crate::ScannerGetObjectReader; +use crate::data_usage_define::DATA_USAGE_BLOOM_RECOVERY_PATH; +use crate::storage_api::owner::ObjectIO as _; +use tokio::io::AsyncReadExt as _; + +const SCANNER_CYCLE_RECOVERY_SCHEMA_VERSION: u16 = 1; +const MAX_SCANNER_CYCLE_STATE_BYTES: u64 = 1024 * 1024; +pub(super) const MAX_SCANNER_CYCLE_RECOVERY_RETRIES: u32 = 5; +const METRIC_SCANNER_CYCLE_RECOVERY_REQUIRED: &str = "rustfs_scanner_cycle_recovery_required"; +const METRIC_SCANNER_CYCLE_RECOVERY_RETRY_COUNT: &str = "rustfs_scanner_cycle_recovery_retry_count"; + +#[derive(Clone, Debug, Default, Serialize)] +pub struct ScannerCycleRecoveryStatus { + /// The immutable primary object whose revision is being guarded. + pub path: String, + /// The companion marker/quarantine object containing the recovery evidence. + pub quarantine_path: Option, + pub state: String, + pub classification: Option, + pub primary_revision: Option, + pub generation: Option, + pub leader_epoch: Option, + pub first_detected_at_unix_secs: Option, + pub last_attempt_at_unix_secs: Option, + pub retry_count: u64, + pub max_retries: u32, + /// Whether the scanner may retry this state automatically. + pub retryable: bool, + pub reason: Option, +} + +static SCANNER_CYCLE_RECOVERY_STATUS: LazyLock> = LazyLock::new(|| { + RwLock::new(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "healthy".to_string(), + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + ..Default::default() + }) +}); + +pub fn scanner_cycle_recovery_status() -> ScannerCycleRecoveryStatus { + SCANNER_CYCLE_RECOVERY_STATUS + .read() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone() +} + +fn set_scanner_cycle_recovery_status(status: ScannerCycleRecoveryStatus) { + let recovery_required = if matches!(status.state.as_str(), "blocked" | "paused" | "recovery-required" | "cleanup-pending") { + 1.0 + } else { + 0.0 + }; + metrics::gauge!(METRIC_SCANNER_CYCLE_RECOVERY_REQUIRED).set(recovery_required); + metrics::gauge!(METRIC_SCANNER_CYCLE_RECOVERY_RETRY_COUNT).set(status.retry_count as f64); + *SCANNER_CYCLE_RECOVERY_STATUS + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = status; +} + +pub(super) fn record_scanner_cycle_recovery_retry(attempt: u32) -> bool { + let mut status = scanner_cycle_recovery_status(); + status.retry_count = u64::from(attempt); + status.last_attempt_at_unix_secs = Some(unix_now_secs()); + if attempt >= MAX_SCANNER_CYCLE_RECOVERY_RETRIES { + status.state = "paused".to_string(); + status.retryable = false; + status.reason = Some("scanner cycle recovery retry budget reached; sparse backend probes continue".to_string()); + set_scanner_cycle_recovery_status(status); + false + } else { + status.retryable = true; + set_scanner_cycle_recovery_status(status); + true + } +} + +fn unix_now_secs() -> u64 { + u64::try_from(Utc::now().timestamp()).unwrap_or(0) +} + +fn recovery_status(state: &str, reason: Option<&str>, retryable: bool) -> ScannerCycleRecoveryStatus { + ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: state.to_string(), + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable, + last_attempt_at_unix_secs: Some(unix_now_secs()), + reason: reason.map(str::to_string), + ..Default::default() + } +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ScannerCycleRecoveryMarker { + pub schema_version: u16, + pub primary_revision: String, + pub generation: u64, + pub leader_epoch: u64, + pub classification: String, + pub first_detected_at_unix_secs: u64, + pub last_attempt_at_unix_secs: u64, + pub retry_count: u64, + pub reason: String, + pub path: String, + pub quarantine_path: String, + /// `blocked` means the marker guards the primary revision; `cleanup-pending` + /// means an operator reset is in progress and must remain fenced across a + /// restart, even if the primary object is subsequently rewritten. + #[serde(default = "default_recovery_marker_state")] + pub state: String, +} + +fn default_recovery_marker_state() -> String { + "blocked".to_string() +} + +#[derive(Debug, Deserialize)] +struct ScannerCycleRecoveryMarkerCompat { + schema_version: Option, + primary_revision: Option, + classification: Option, + first_detected_at_unix_secs: Option, + last_attempt_at_unix_secs: Option, + retry_count: Option, + reason: Option, + path: Option, + quarantine_path: Option, + state: Option, +} + +#[derive(Debug)] +pub(crate) enum ScannerCycleStateStartup { + Ready { + cycle: CurrentCycle, + leader_epoch: u64, + revision: DataUsageCacheRevision, + }, + Blocked, + Transient(ScannerError), +} + +#[derive(Debug, thiserror::Error)] +enum CycleRecoveryMarkerReadError { + #[error("cycle recovery marker backend read failed: {0}")] + Backend(#[source] EcstoreError), + #[error("invalid cycle recovery marker: {0}")] + Invalid(&'static str), + #[error("cycle recovery marker revision changed while publishing")] + Conflict, +} + +#[derive(Debug, thiserror::Error)] +enum CycleStateBodyReadError { + #[error("scanner cycle state exceeds the bounded object size")] + TooLarge, + #[error("scanner cycle state body read failed: {0}")] + Backend(#[source] EcstoreError), +} + +fn recovery_status_from_marker(marker: &ScannerCycleRecoveryMarker, state: &str) -> ScannerCycleRecoveryStatus { + ScannerCycleRecoveryStatus { + path: marker.path.clone(), + quarantine_path: Some(marker.quarantine_path.clone()), + state: state.to_string(), + classification: Some(marker.classification.clone()), + primary_revision: Some(marker.primary_revision.clone()), + generation: Some(marker.generation), + leader_epoch: Some(marker.leader_epoch), + first_detected_at_unix_secs: Some(marker.first_detected_at_unix_secs), + last_attempt_at_unix_secs: Some(marker.last_attempt_at_unix_secs), + retry_count: marker.retry_count, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: false, + reason: Some(marker.reason.clone()), + } +} + +fn marker_matches_revision(marker: &ScannerCycleRecoveryMarker, revision: &DataUsageCacheRevision) -> bool { + matches!(revision, DataUsageCacheRevision::Etag(etag) if marker.primary_revision == *etag) +} + +fn validate_recovery_marker(marker: &ScannerCycleRecoveryMarker) -> Result<(), &'static str> { + if marker.schema_version != SCANNER_CYCLE_RECOVERY_SCHEMA_VERSION { + return Err("cycle recovery marker schema is unsupported"); + } + if marker.primary_revision.is_empty() { + return Err("cycle recovery marker has no primary revision"); + } + if marker.path != *DATA_USAGE_BLOOM_NAME_PATH { + return Err("cycle recovery marker path does not match the scanner scope"); + } + if marker.quarantine_path != *DATA_USAGE_BLOOM_RECOVERY_PATH { + return Err("cycle recovery marker quarantine path does not match the scanner scope"); + } + if !matches!(marker.classification.as_str(), "corrupt" | "future_schema") { + return Err("cycle recovery marker classification is invalid"); + } + if !matches!(marker.state.as_str(), "blocked" | "cleanup-pending") { + return Err("cycle recovery marker state is invalid"); + } + Ok(()) +} + +/// Decode only the stable scope and revision fields needed by an authenticated +/// full-rescan reset. Startup keeps the strict decoder above so a newer marker +/// cannot be interpreted as a trusted cursor; reset deliberately rebuilds from +/// the persisted usage floor instead. +pub(super) fn decode_recovery_marker_for_reset( + data: &[u8], + marker_revision: &DataUsageCacheRevision, +) -> Result { + if !matches!(marker_revision, DataUsageCacheRevision::Etag(_)) { + return Err(ScannerError::Other("cycle recovery marker has no object revision".to_string())); + } + let compat = serde_json::from_slice::(data).ok(); + let _schema_version = compat.as_ref().and_then(|marker| marker.schema_version); + let primary_revision = compat + .as_ref() + .and_then(|marker| marker.primary_revision.clone()) + .filter(|revision| !revision.is_empty()) + .unwrap_or_default(); + let path = compat + .as_ref() + .and_then(|marker| marker.path.clone()) + .unwrap_or_else(|| DATA_USAGE_BLOOM_NAME_PATH.clone()); + let quarantine_path = compat + .as_ref() + .and_then(|marker| marker.quarantine_path.clone()) + .unwrap_or_else(|| DATA_USAGE_BLOOM_RECOVERY_PATH.clone()); + if path != *DATA_USAGE_BLOOM_NAME_PATH || quarantine_path != *DATA_USAGE_BLOOM_RECOVERY_PATH { + return Err(ScannerError::Other( + "cycle recovery marker path does not match the scanner scope".to_string(), + )); + } + let classification = match compat.as_ref().and_then(|marker| marker.classification.as_deref()) { + Some("corrupt") => "corrupt", + Some("future_schema") | None => "future_schema", + Some(_) => "future_schema", + }; + let state = match compat.as_ref().and_then(|marker| marker.state.as_deref()) { + Some("cleanup-pending") => "cleanup-pending", + _ => "blocked", + }; + let now = unix_now_secs(); + Ok(ScannerCycleRecoveryMarker { + schema_version: SCANNER_CYCLE_RECOVERY_SCHEMA_VERSION, + primary_revision, + // Cursor and epoch values from an unknown marker are audit-only data; + // the reset path intentionally rebuilds both from the verified usage + // floor instead of carrying them across a version boundary. + generation: 0, + leader_epoch: 0, + classification: classification.to_string(), + first_detected_at_unix_secs: compat + .as_ref() + .and_then(|marker| marker.first_detected_at_unix_secs) + .unwrap_or(now), + last_attempt_at_unix_secs: compat + .as_ref() + .and_then(|marker| marker.last_attempt_at_unix_secs) + .unwrap_or(now), + retry_count: compat.as_ref().and_then(|marker| marker.retry_count).unwrap_or(0), + reason: compat + .as_ref() + .and_then(|marker| marker.reason.clone()) + .unwrap_or_else(|| "operator requested full scanner rescan".to_string()), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: state.to_string(), + }) +} + +async fn read_cycle_state_body(reader: &mut ScannerGetObjectReader) -> Result, CycleStateBodyReadError> { + let max_len = usize::try_from(MAX_SCANNER_CYCLE_STATE_BYTES).unwrap_or(usize::MAX); + let mut data = Vec::new(); + reader + .take(MAX_SCANNER_CYCLE_STATE_BYTES.saturating_add(1)) + .read_to_end(&mut data) + .await + .map_err(|err| CycleStateBodyReadError::Backend(EcstoreError::other(err)))?; + if data.len() > max_len { + return Err(CycleStateBodyReadError::TooLarge); + } + Ok(data) +} + +fn cycle_state_classification(buf: &[u8]) -> (&'static str, &'static str) { + if buf.len() >= 16 && &buf[8..12] == b"RSCY" && &buf[8..16] != SCANNER_CYCLE_STATE_MAGIC { + ("future_schema", "scanner cycle state schema is newer than this reader") + } else { + ("corrupt", "scanner cycle state failed validation") + } +} + +fn cycle_state_generation_and_epoch(buf: &[u8]) -> (u64, u64) { + let generation = buf + .get(..8) + .and_then(|bytes| bytes.try_into().ok()) + .map(u64::from_le_bytes) + .unwrap_or(0); + let leader_epoch = if buf.len() >= SCANNER_CYCLE_STATE_HEADER_LEN && &buf[8..16] == SCANNER_CYCLE_STATE_MAGIC { + u64::from_le_bytes(buf[16..24].try_into().unwrap_or([0; 8])) + } else { + 0 + }; + (generation, leader_epoch) +} + +async fn persist_cycle_recovery_marker( + storeapi: Arc, + primary_revision: &DataUsageCacheRevision, + generation: u64, + leader_epoch: u64, + classification: &'static str, + reason: &'static str, +) -> Result { + let now = unix_now_secs(); + let (existing, existing_revision) = match read_cycle_recovery_marker_bytes(storeapi.clone()).await { + Ok(result) => result, + Err(err) => return Err(err), + }; + let existing_marker = existing + .as_deref() + .and_then(|bytes| serde_json::from_slice::(bytes).ok()); + let primary_revision = match primary_revision { + DataUsageCacheRevision::Etag(etag) => etag.clone(), + DataUsageCacheRevision::Missing => { + return Err(CycleRecoveryMarkerReadError::Invalid("cycle state recovery requires a primary revision")); + } + }; + let marker = ScannerCycleRecoveryMarker { + schema_version: SCANNER_CYCLE_RECOVERY_SCHEMA_VERSION, + primary_revision: primary_revision.clone(), + generation, + leader_epoch, + classification: classification.to_string(), + first_detected_at_unix_secs: existing_marker + .as_ref() + .filter(|marker| marker.primary_revision == primary_revision) + .map(|marker| marker.first_detected_at_unix_secs) + .unwrap_or(now), + last_attempt_at_unix_secs: now, + retry_count: existing_marker + .as_ref() + .filter(|marker| marker.primary_revision == primary_revision) + .map(|marker| marker.retry_count.saturating_add(1)) + .unwrap_or(0), + reason: reason.to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "blocked".to_string(), + }; + let bytes = serde_json::to_vec(&marker).map_err(|_| CycleRecoveryMarkerReadError::Invalid("marker serialization failed"))?; + let save_result = save_config_with_preconditions( + storeapi.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + bytes, + existing_revision.preconditions(), + ) + .await; + match save_result { + Ok(_) => Ok(marker), + Err(EcstoreError::PreconditionFailed) => Err(CycleRecoveryMarkerReadError::Conflict), + Err(err) => Err(CycleRecoveryMarkerReadError::Backend(err)), + } +} + +async fn read_cycle_recovery_marker_bytes( + storeapi: Arc, +) -> Result<(Option>, DataUsageCacheRevision), CycleRecoveryMarkerReadError> { + let mut reader = match storeapi + .get_object_reader( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + None, + http::HeaderMap::new(), + &ScannerObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(reader) => reader, + Err( + EcstoreError::FileNotFound + | EcstoreError::VolumeNotFound + | EcstoreError::ObjectNotFound(_, _) + | EcstoreError::BucketNotFound(_) + | EcstoreError::ConfigNotFound, + ) => { + return Ok((None, DataUsageCacheRevision::Missing)); + } + Err(err) => return Err(CycleRecoveryMarkerReadError::Backend(err)), + }; + let revision = reader + .object_info + .etag + .as_ref() + .filter(|etag| !etag.is_empty()) + .cloned() + .map(DataUsageCacheRevision::Etag) + .ok_or(CycleRecoveryMarkerReadError::Invalid("marker has no revision"))?; + if reader.object_info.is_dir || reader.object_info.size < 0 || reader.object_info.size > 64 * 1024 { + return Err(CycleRecoveryMarkerReadError::Invalid("marker exceeds the bounded object size")); + } + let mut data = Vec::new(); + (&mut reader) + .take(64 * 1024 + 1) + .read_to_end(&mut data) + .await + .map_err(|err| CycleRecoveryMarkerReadError::Backend(EcstoreError::other(err)))?; + if data.len() > 64 * 1024 { + return Err(CycleRecoveryMarkerReadError::Invalid("marker exceeds the bounded object size")); + } + if data.is_empty() { + return Err(CycleRecoveryMarkerReadError::Invalid("marker is empty")); + } + Ok((Some(data), revision)) +} + +async fn read_cycle_recovery_marker_revision( + storeapi: Arc, +) -> Result { + let reader = match storeapi + .get_object_reader( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + None, + http::HeaderMap::new(), + &ScannerObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(reader) => reader, + Err( + EcstoreError::FileNotFound + | EcstoreError::VolumeNotFound + | EcstoreError::ObjectNotFound(_, _) + | EcstoreError::BucketNotFound(_) + | EcstoreError::ConfigNotFound, + ) => return Ok(DataUsageCacheRevision::Missing), + Err(err) => return Err(CycleRecoveryMarkerReadError::Backend(err)), + }; + if reader.object_info.is_dir || reader.object_info.size < 0 { + return Err(CycleRecoveryMarkerReadError::Invalid("marker is not a regular object")); + } + reader + .object_info + .etag + .as_ref() + .filter(|etag| !etag.is_empty()) + .cloned() + .map(DataUsageCacheRevision::Etag) + .ok_or(CycleRecoveryMarkerReadError::Invalid("marker has no revision")) +} + +async fn quarantine_invalid_cycle_state( + storeapi: Arc, + revision: &DataUsageCacheRevision, + buf: &[u8], +) -> ScannerCycleStateStartup { + let (classification, reason) = cycle_state_classification(buf); + let (generation, leader_epoch) = cycle_state_generation_and_epoch(buf); + quarantine_invalid_cycle_state_with_reason(storeapi, revision, generation, leader_epoch, classification, reason).await +} + +async fn quarantine_invalid_cycle_state_with_reason( + storeapi: Arc, + revision: &DataUsageCacheRevision, + generation: u64, + leader_epoch: u64, + classification: &'static str, + reason: &'static str, +) -> ScannerCycleStateStartup { + let now = unix_now_secs(); + let base_status = ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "recovery-required".to_string(), + classification: Some(classification.to_string()), + primary_revision: match revision { + DataUsageCacheRevision::Etag(etag) => Some(etag.clone()), + DataUsageCacheRevision::Missing => None, + }, + generation: Some(generation), + leader_epoch: Some(leader_epoch), + first_detected_at_unix_secs: Some(now), + last_attempt_at_unix_secs: Some(now), + retry_count: 0, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: true, + reason: Some(reason.to_string()), + }; + set_scanner_cycle_recovery_status(base_status); + match persist_cycle_recovery_marker(storeapi, revision, generation, leader_epoch, classification, reason).await { + Ok(marker) => set_scanner_cycle_recovery_status(recovery_status_from_marker(&marker, "blocked")), + Err(CycleRecoveryMarkerReadError::Backend(_)) => { + // Keep the poison object untouched and retry marker creation with the + // bounded startup backoff; recovery-required never becomes healthy. + return ScannerCycleStateStartup::Transient(ScannerError::Other( + "failed to persist scanner cycle recovery marker".to_string(), + )); + } + Err(CycleRecoveryMarkerReadError::Conflict) => { + set_scanner_cycle_recovery_status(recovery_status( + "transient", + Some("cycle recovery marker revision changed while publishing"), + true, + )); + return ScannerCycleStateStartup::Transient(ScannerError::Other( + "cycle recovery marker revision changed while publishing".to_string(), + )); + } + Err(CycleRecoveryMarkerReadError::Invalid(reason)) => { + set_scanner_cycle_recovery_status(recovery_status("recovery-required", Some(reason), false)); + return ScannerCycleStateStartup::Blocked; + } + } + ScannerCycleStateStartup::Blocked +} + +async fn mark_cycle_recovery_cleanup_pending( + storeapi: Arc, + mut marker: ScannerCycleRecoveryMarker, + marker_revision: &DataUsageCacheRevision, +) -> Result<(ScannerCycleRecoveryMarker, DataUsageCacheRevision), ScannerError> { + marker.state = "cleanup-pending".to_string(); + marker.last_attempt_at_unix_secs = unix_now_secs(); + let bytes = serde_json::to_vec(&marker) + .map_err(|err| ScannerError::Other(format!("failed to encode cycle recovery marker: {err}")))?; + let info = save_config_with_preconditions( + storeapi.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + bytes, + marker_revision.preconditions(), + ) + .await + .map_err(|err| ScannerError::Other(format!("failed to mark cycle recovery cleanup pending: {err}")))?; + let revision = info + .etag + .filter(|etag| !etag.is_empty()) + .map(DataUsageCacheRevision::Etag) + .ok_or_else(|| ScannerError::Other("cycle recovery marker save returned no revision".to_string()))?; + Ok((marker, revision)) +} + +pub(crate) async fn load_scanner_cycle_state_for_startup(storeapi: Arc) -> ScannerCycleStateStartup { + let marker = match read_cycle_recovery_marker_bytes(storeapi.clone()).await { + Ok((None, _)) => None, + Ok((Some(data), marker_revision)) => match serde_json::from_slice::(&data) { + Ok(marker) => match validate_recovery_marker(&marker) { + Ok(()) => Some((marker, marker_revision)), + Err(reason) => { + set_scanner_cycle_recovery_status(recovery_status("recovery-required", Some(reason), false)); + return ScannerCycleStateStartup::Blocked; + } + }, + Err(_) => { + set_scanner_cycle_recovery_status(recovery_status( + "recovery-required", + Some("cycle recovery marker is invalid"), + false, + )); + return ScannerCycleStateStartup::Blocked; + } + }, + Err(CycleRecoveryMarkerReadError::Backend(err)) => { + let status = recovery_status("transient", Some("cycle recovery marker I/O is temporarily unavailable"), true); + set_scanner_cycle_recovery_status(status); + return ScannerCycleStateStartup::Transient(ScannerError::Other(format!( + "failed to read scanner cycle recovery marker: {err}" + ))); + } + Err(CycleRecoveryMarkerReadError::Invalid(reason)) => { + set_scanner_cycle_recovery_status(recovery_status("recovery-required", Some(reason), false)); + return ScannerCycleStateStartup::Blocked; + } + Err(CycleRecoveryMarkerReadError::Conflict) => { + set_scanner_cycle_recovery_status(recovery_status( + "transient", + Some("cycle recovery marker revision changed while being inspected"), + true, + )); + return ScannerCycleStateStartup::Transient(ScannerError::Other( + "cycle recovery marker revision changed while being inspected".to_string(), + )); + } + }; + + let mut reader = match storeapi + .get_object_reader( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + None, + http::HeaderMap::new(), + &ScannerObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(reader) => reader, + Err( + EcstoreError::FileNotFound + | EcstoreError::VolumeNotFound + | EcstoreError::ObjectNotFound(_, _) + | EcstoreError::BucketNotFound(_) + | EcstoreError::ConfigNotFound, + ) => { + if let Some((marker, _)) = marker { + let state = if marker.state == "cleanup-pending" { + "cleanup-pending" + } else { + "recovery-required" + }; + set_scanner_cycle_recovery_status(recovery_status_from_marker(&marker, state)); + return ScannerCycleStateStartup::Blocked; + } + set_scanner_cycle_recovery_status(recovery_status("healthy", None, false)); + return ScannerCycleStateStartup::Ready { + cycle: CurrentCycle::default(), + leader_epoch: 0, + revision: DataUsageCacheRevision::Missing, + }; + } + Err(err) => { + set_scanner_cycle_recovery_status(recovery_status("transient", Some("cycle state could not be inspected"), true)); + return ScannerCycleStateStartup::Transient(ScannerError::Other(format!( + "failed to inspect scanner cycle state: {err}" + ))); + } + }; + let revision = reader + .object_info + .etag + .as_ref() + .filter(|etag| !etag.is_empty()) + .cloned() + .map(DataUsageCacheRevision::Etag); + let Some(revision) = revision else { + set_scanner_cycle_recovery_status(recovery_status("recovery-required", Some("cycle state has no revision"), false)); + return ScannerCycleStateStartup::Blocked; + }; + let max_size = i64::try_from(MAX_SCANNER_CYCLE_STATE_BYTES).unwrap_or(i64::MAX); + if reader.object_info.is_dir || reader.object_info.size < 0 || reader.object_info.size > max_size { + return quarantine_invalid_cycle_state_with_reason( + storeapi, + &revision, + 0, + 0, + "corrupt", + "scanner cycle state object is oversized or not a regular object", + ) + .await; + } + if let Some((marker, _)) = marker + .as_ref() + .filter(|(marker, _)| marker.state == "cleanup-pending" || marker_matches_revision(marker, &revision)) + { + let state = if marker.state == "cleanup-pending" { + "cleanup-pending" + } else { + "blocked" + }; + set_scanner_cycle_recovery_status(recovery_status_from_marker(marker, state)); + return ScannerCycleStateStartup::Blocked; + } + let data = match read_cycle_state_body(&mut reader).await { + Ok(data) => data, + Err(CycleStateBodyReadError::TooLarge) => { + return quarantine_invalid_cycle_state_with_reason( + storeapi, + &revision, + 0, + 0, + "corrupt", + "scanner cycle state exceeds the bounded object size", + ) + .await; + } + Err(CycleStateBodyReadError::Backend(err)) => { + set_scanner_cycle_recovery_status(recovery_status("transient", Some("cycle state read failed"), true)); + return ScannerCycleStateStartup::Transient(ScannerError::Other(format!( + "failed to read scanner cycle state: {err}" + ))); + } + }; + if data.is_empty() { + return quarantine_invalid_cycle_state_with_reason( + storeapi, + &revision, + 0, + 0, + "corrupt", + "scanner cycle state object is empty", + ) + .await; + } + match decode_scanner_cycle_state_for_startup(&data) { + Ok((cycle, leader_epoch)) => { + set_scanner_cycle_recovery_status(recovery_status("healthy", None, false)); + ScannerCycleStateStartup::Ready { + cycle, + leader_epoch, + revision, + } + } + Err(_) => quarantine_invalid_cycle_state(storeapi, &revision, &data).await, + } +} + +/// Reset a blocked cycle state after an operator has explicitly requested a full +/// usage rebuild. The primary object is changed first with its observed ETag; +/// the recovery marker is removed only when its own ETag still matches. +pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc) -> Result<(), ScannerError> { + let lock = storeapi + .new_ns_lock(RUSTFS_META_BUCKET, "leader.lock") + .await + .map_err(|err| ScannerError::Other(format!("failed to acquire scanner leader lock: {err}")))?; + let guard = lock + .get_write_lock_quiet(Duration::from_secs(5)) + .await + .map_err(|err| ScannerError::Other(format!("scanner leader lock is busy: {err}")))?; + + if guard.is_lock_lost() { + return Err(ScannerError::Other("scanner leader lock was lost before recovery reset".to_string())); + } + + let (marker_data, marker_revision, marker_body_invalid) = match read_cycle_recovery_marker_bytes(storeapi.clone()).await { + Ok((marker_data, marker_revision)) => (marker_data, marker_revision, false), + Err(CycleRecoveryMarkerReadError::Invalid(_)) => { + let marker_revision = read_cycle_recovery_marker_revision(storeapi.clone()) + .await + .map_err(|err| ScannerError::Other(format!("failed to read cycle recovery marker: {err}")))?; + (Some(Vec::new()), marker_revision, true) + } + Err(err) => return Err(ScannerError::Other(format!("failed to read cycle recovery marker: {err}"))), + }; + let marker_data = marker_data.ok_or_else(|| ScannerError::Other("scanner cycle recovery marker is absent".to_string()))?; + let (marker, force_full_rescan) = match serde_json::from_slice::(&marker_data) { + Ok(marker) if validate_recovery_marker(&marker).is_ok() => (marker, false), + _ => (decode_recovery_marker_for_reset(&marker_data, &marker_revision)?, true), + }; + let force_full_rescan = force_full_rescan || marker_body_invalid; + + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost while reading recovery state".to_string(), + )); + } + + let (mut primary_reader, primary_revision) = match storeapi + .get_object_reader( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + None, + http::HeaderMap::new(), + &ScannerObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(reader) => { + let revision = reader + .object_info + .etag + .as_ref() + .filter(|etag| !etag.is_empty()) + .cloned() + .ok_or_else(|| ScannerError::Other("scanner cycle state has no revision".to_string()))?; + (Some(reader), DataUsageCacheRevision::Etag(revision)) + } + Err( + EcstoreError::FileNotFound + | EcstoreError::VolumeNotFound + | EcstoreError::ObjectNotFound(_, _) + | EcstoreError::BucketNotFound(_) + | EcstoreError::ConfigNotFound, + ) => (None, DataUsageCacheRevision::Missing), + Err(err) => return Err(ScannerError::Other(format!("failed to inspect scanner cycle state: {err}"))), + }; + let marker_cleanup_pending = marker.state == "cleanup-pending"; + let marker_matches_primary = marker_matches_revision(&marker, &primary_revision); + if (marker_cleanup_pending || !marker_matches_primary) + && let Some(mut reader) = primary_reader.take() + { + // A newer, independently fenced primary is authoritative. A + // full-rescan reset must not overwrite that progress; it only + // removes the stale recovery marker after validating and re-fencing + // the state. + let max_size = i64::try_from(MAX_SCANNER_CYCLE_STATE_BYTES).unwrap_or(i64::MAX); + if reader.object_info.is_dir || reader.object_info.size < 0 { + return Err(ScannerError::Other("scanner cycle state changed since recovery was recorded".to_string())); + } + let primary_is_oversized = reader.object_info.size > max_size; + let primary_state = if primary_is_oversized { + None + } else { + match read_cycle_state_body(&mut reader).await { + Ok(data) if data.is_empty() => None, + Ok(data) => decode_scanner_cycle_state_for_startup(&data).ok(), + Err(CycleStateBodyReadError::TooLarge) if force_full_rescan || marker_cleanup_pending => None, + Err(err) => { + return Err(ScannerError::Other(format!( + "scanner cycle state changed since recovery was recorded: {err}" + ))); + } + } + }; + if let Some((primary_cycle, primary_epoch)) = primary_state { + let (cleanup_marker, cleanup_marker_revision) = + mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker.clone(), &marker_revision).await?; + set_scanner_cycle_recovery_status(recovery_status_from_marker(&cleanup_marker, "cleanup-pending")); + let usage_floor = persisted_usage_floor(storeapi.clone()).await?; + let fence_epoch = primary_epoch + .max(usage_floor.leader_epoch) + .checked_add(1) + .filter(|epoch| *epoch < u64::MAX) + .ok_or_else(|| ScannerError::Other("scanner leader epoch is exhausted".to_string()))?; + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost before preserving newer cycle state".to_string(), + )); + } + let preserved_data = encode_scanner_cycle_state(&primary_cycle, fence_epoch) + .map_err(|err| ScannerError::Other(format!("failed to encode preserved scanner cycle state: {err}")))?; + if u64::try_from(preserved_data.len()).unwrap_or(u64::MAX) > MAX_SCANNER_CYCLE_STATE_BYTES { + return Err(ScannerError::Other( + "preserved scanner cycle state exceeds the bounded object size".to_string(), + )); + } + let preserved_info = save_config_with_preconditions( + storeapi.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + preserved_data, + primary_revision.preconditions(), + ) + .await + .map_err(|err| ScannerError::Other(format!("failed to fence preserved scanner cycle state: {err}")))?; + let preserved_revision = preserved_info + .etag + .filter(|etag| !etag.is_empty()) + .map(DataUsageCacheRevision::Etag) + .ok_or_else(|| ScannerError::Other("preserved scanner cycle state has no revision".to_string()))?; + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost after fencing newer cycle state".to_string(), + )); + } + fence_scanner_usage_epoch(&ctx, storeapi.clone(), fence_epoch) + .await + .map_err(|err| ScannerError::Other(format!("failed to fence preserved scanner usage epoch: {err}")))?; + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost after fencing newer cycle state".to_string(), + )); + } + let current_revision = read_config_revision(storeapi.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to verify preserved scanner cycle state: {err}")))?; + if current_revision != preserved_revision { + return Err(ScannerError::Other( + "scanner cycle state changed before recovery marker cleanup".to_string(), + )); + } + storeapi + .delete_config_object( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + ScannerObjectOptions { + // This is one exact metadata object. Prefix-delete mode + // bypasses HTTP preconditions in the ECStore path. + delete_prefix: false, + http_preconditions: Some(cleanup_marker_revision.preconditions()), + ..Default::default() + }, + ) + .await + .map_err(|err| ScannerError::Other(format!("failed to clear stale cycle recovery marker: {err}")))?; + set_scanner_cycle_recovery_status(recovery_status("healthy", None, false)); + super::notify_scanner_cycle_recovery_wake(); + return Ok(()); + } else if !force_full_rescan && !marker_cleanup_pending { + // An invalid compatibility marker cannot fence a corrupt primary + // by revision, so rebuild it from the verified usage floor below. + // A strict marker keeps the existing fail-closed behavior for an + // unexpected stale-primary mutation. + return Err(ScannerError::Other("scanner cycle state changed since recovery was recorded".to_string())); + } + } + + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost before rebuilding cycle state".to_string(), + )); + } + + let floor = persisted_usage_floor(storeapi.clone()).await?; + // A full rescan must not trust a cursor recovered from a corrupt, future, + // or mixed-version marker. The durable usage floor is the only verified + // starting point; marker generation/epoch fields remain audit evidence. + let next = floor.next_cycle; + if next == u64::MAX { + return Err(ScannerError::Other("scanner cycle counter is exhausted".to_string())); + } + let leader_epoch = floor + .leader_epoch + .checked_add(1) + .filter(|epoch| *epoch < u64::MAX) + .ok_or_else(|| ScannerError::Other("scanner leader epoch is exhausted".to_string()))?; + let cycle = CurrentCycle { + next, + ..Default::default() + }; + let data = encode_scanner_cycle_state(&cycle, leader_epoch) + .map_err(|err| ScannerError::Other(format!("failed to encode rebuilt scanner cycle state: {err}")))?; + // Persist the cleanup-pending phase before rewriting the primary. If the + // process dies after the rewrite, startup still sees a durable fence and + // cannot mistake the partially completed reset for a healthy state. + let (marker, marker_revision) = if marker.state == "cleanup-pending" { + (marker, marker_revision) + } else { + mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker, &marker_revision).await? + }; + let rebuilt_info = save_config_with_preconditions( + storeapi.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + data, + primary_revision.preconditions(), + ) + .await + .map_err(|err| ScannerError::Other(format!("failed to persist rebuilt scanner cycle state: {err}")))?; + let rebuilt_revision = rebuilt_info + .etag + .filter(|etag| !etag.is_empty()) + .ok_or_else(|| ScannerError::Other("rebuilt scanner cycle state has no revision".to_string()))?; + if guard.is_lock_lost() { + return Err(ScannerError::Other( + "scanner leader lock was lost after rebuilding cycle state".to_string(), + )); + } + if let Err(err) = fence_scanner_usage_epoch(&ctx, storeapi.clone(), leader_epoch).await { + set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "cleanup-pending".to_string(), + classification: Some(marker.classification.clone()), + primary_revision: Some(rebuilt_revision.clone()), + generation: Some(next), + leader_epoch: Some(leader_epoch), + first_detected_at_unix_secs: Some(marker.first_detected_at_unix_secs), + last_attempt_at_unix_secs: Some(unix_now_secs()), + retry_count: marker.retry_count, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: false, + reason: Some("cycle state rebuilt but usage epoch fencing failed".to_string()), + }); + return Err(err); + } + + let current_revision = match read_config_revision(storeapi.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to verify rebuilt scanner cycle state: {err}")))? + { + DataUsageCacheRevision::Etag(etag) => etag, + DataUsageCacheRevision::Missing => { + return Err(ScannerError::Other("rebuilt scanner cycle state lost its revision".to_string())); + } + }; + if current_revision != rebuilt_revision { + set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "cleanup-pending".to_string(), + classification: Some(marker.classification.clone()), + primary_revision: Some(current_revision), + generation: Some(next), + leader_epoch: Some(leader_epoch), + first_detected_at_unix_secs: Some(marker.first_detected_at_unix_secs), + last_attempt_at_unix_secs: Some(unix_now_secs()), + retry_count: marker.retry_count, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: false, + reason: Some("rebuilt scanner cycle state changed before marker cleanup".to_string()), + }); + return Err(ScannerError::Other( + "rebuilt scanner cycle state changed before recovery marker cleanup".to_string(), + )); + } + + if guard.is_lock_lost() { + set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "cleanup-pending".to_string(), + classification: Some(marker.classification.clone()), + primary_revision: Some(rebuilt_revision.clone()), + generation: Some(next), + leader_epoch: Some(leader_epoch), + retry_count: marker.retry_count, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: false, + reason: Some("cycle state rebuilt but recovery marker was not cleared".to_string()), + ..Default::default() + }); + return Err(ScannerError::Other( + "scanner leader lock was lost before clearing recovery marker".to_string(), + )); + } + + if let Err(err) = storeapi + .delete_config_object( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + ScannerObjectOptions { + // This is one exact metadata object. Prefix-delete mode + // bypasses HTTP preconditions in the ECStore path. + delete_prefix: false, + http_preconditions: Some(marker_revision.preconditions()), + ..Default::default() + }, + ) + .await + { + set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "cleanup-pending".to_string(), + classification: Some(marker.classification.clone()), + primary_revision: Some(rebuilt_revision.clone()), + generation: Some(next), + leader_epoch: Some(leader_epoch), + retry_count: marker.retry_count, + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + retryable: false, + reason: Some("cycle state rebuilt but recovery marker cleanup failed".to_string()), + ..Default::default() + }); + return Err(ScannerError::Other(format!("failed to clear cycle recovery marker: {err}"))); + } + set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: Some(DATA_USAGE_BLOOM_RECOVERY_PATH.clone()), + state: "healthy".to_string(), + max_retries: MAX_SCANNER_CYCLE_RECOVERY_RETRIES, + ..Default::default() + }); + super::notify_scanner_cycle_recovery_wake(); + Ok(()) +} #[derive(Debug, thiserror::Error)] pub(super) enum ScannerCycleStateError { @@ -86,7 +1147,11 @@ pub(super) fn decode_scanner_cycle_state(buf: &[u8]) -> Result<(CurrentCycle, u6 (0, &buf[8..]) }; - let cycle_info = rmp_serde::from_slice::(payload)?; + let mut deserializer = rmp_serde::Deserializer::new(std::io::Cursor::new(payload)); + let cycle_info = CurrentCycle::deserialize(&mut deserializer)?; + if deserializer.position() != u64::try_from(payload.len()).unwrap_or(u64::MAX) { + return Err(ScannerCycleStateError::InvalidData("scanner cycle state has trailing bytes")); + } if cycle_info.next != persisted_next { return Err(ScannerCycleStateError::InvalidData("scanner cycle counter disagrees with encoded state")); } @@ -146,7 +1211,7 @@ pub(super) fn advance_scanner_cycle(cycle_info: &mut CurrentCycle) -> Result<(), pub(super) async fn persisted_usage_floor(storeapi: Arc) -> Result { let mut floor = PersistedUsageFloor::default(); - let update_floor = |floor: &mut PersistedUsageFloor, usage: DataUsageInfo, path: &str| -> Result<(), ScannerError> { + let update_floor = |floor: &mut PersistedUsageFloor, usage: &DataUsageInfo, path: &str| -> Result<(), ScannerError> { floor.leader_epoch = floor.leader_epoch.max(usage.scanner_epoch.unwrap_or_default()); if let Some(completed_cycle) = usage.scanner_cycle { let next_cycle = completed_cycle @@ -159,25 +1224,45 @@ pub(super) async fn persisted_usage_floor(storeapi: Arc) - }; for primary_path in [DATA_USAGE_OBJ_NAME_PATH.as_str(), LEGACY_DATA_USAGE_OBJ_NAME_PATH.as_str()] { let backup_path = format!("{primary_path}.bkp"); - let mut pair_found = false; - for path in [primary_path, backup_path.as_str()] { - let data = match read_config(storeapi.clone(), path).await { - Ok(data) => { - pair_found = true; - data + let primary_epoch = match read_config(storeapi.clone(), primary_path).await { + Ok(data) => { + let usage = serde_json::from_slice::(&data).map_err(|err| { + ScannerError::Other(format!("failed to decode scanner usage floor from {primary_path}: {err}")) + })?; + let epoch = usage.scanner_epoch.unwrap_or_default(); + update_floor(&mut floor, &usage, primary_path)?; + Some(epoch) + } + Err(EcstoreError::ConfigNotFound) => None, + Err(err) => { + return Err(ScannerError::Other(format!( + "failed to read scanner usage epoch floor from {primary_path}: {err}" + ))); + } + }; + let mut any_found = primary_epoch.is_some(); + match read_config(storeapi.clone(), &backup_path).await { + Ok(data) => { + any_found = true; + let usage = serde_json::from_slice::(&data).map_err(|err| { + ScannerError::Other(format!("failed to decode scanner usage floor from {backup_path}: {err}")) + })?; + let backup_epoch = usage.scanner_epoch.unwrap_or_default(); + // A backup write from an older leader may complete after the + // primary epoch has been fenced. It must not advance the startup + // floor unless its epoch is at least as new as the primary. + if primary_epoch.is_none_or(|epoch| backup_epoch >= epoch) { + update_floor(&mut floor, &usage, &backup_path)?; } - Err(EcstoreError::ConfigNotFound) => continue, - Err(err) => { - return Err(ScannerError::Other(format!( - "failed to read scanner usage epoch floor from {path}: {err}" - ))); - } - }; - let usage = serde_json::from_slice::(&data) - .map_err(|err| ScannerError::Other(format!("failed to decode scanner usage floor from {path}: {err}")))?; - update_floor(&mut floor, usage, path)?; + } + Err(EcstoreError::ConfigNotFound) => {} + Err(err) => { + return Err(ScannerError::Other(format!( + "failed to read scanner usage epoch floor from {backup_path}: {err}" + ))); + } } - if pair_found { + if any_found { break; } } diff --git a/crates/scanner/src/scanner/leadership.rs b/crates/scanner/src/scanner/leadership.rs index 0ac948549..ab22f56d9 100644 --- a/crates/scanner/src/scanner/leadership.rs +++ b/crates/scanner/src/scanner/leadership.rs @@ -196,7 +196,7 @@ pub(super) async fn claim_scanner_leadership( if ctx.is_cancelled() { return false; } - let Some(claimed_epoch) = persisted_epoch.checked_add(1) else { + let Some(claimed_epoch) = persisted_epoch.checked_add(1).filter(|epoch| *epoch < u64::MAX) else { error!( target: "rustfs::scanner", event = EVENT_SCANNER_PERSIST_STATE, diff --git a/crates/scanner/src/scanner/tests.rs b/crates/scanner/src/scanner/tests.rs index c40f91849..5430e9d29 100644 --- a/crates/scanner/src/scanner/tests.rs +++ b/crates/scanner/src/scanner/tests.rs @@ -15,11 +15,12 @@ use super::*; use crate::EcstoreResult; use crate::{ - Endpoint, EndpointServerPools, Endpoints, InstanceContext, PoolEndpoints, ScannerGetObjectReader as GetObjectReader, - ScannerObjectInfo as ObjectInfo, ScannerObjectOptions as ObjectOptions, ScannerPutObjReader as PutObjReader, - init_bucket_metadata_sys_for_scanner_tests, init_ecstore_config_for_scanner_tests, init_local_disks_with_instance_ctx, + DATA_USAGE_BLOOM_RECOVERY_PATH, Endpoint, EndpointServerPools, Endpoints, InstanceContext, PoolEndpoints, + ScannerGetObjectReader as GetObjectReader, ScannerObjectInfo as ObjectInfo, ScannerObjectOptions as ObjectOptions, + ScannerPutObjReader as PutObjReader, init_bucket_metadata_sys_for_scanner_tests, init_ecstore_config_for_scanner_tests, + init_local_disks_with_instance_ctx, }; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::Cursor; use std::task::Poll; use temp_env::{with_var, with_var_unset}; @@ -117,6 +118,15 @@ async fn scanner_cycle_lock_fence_bounds_uncooperative_shutdown() { assert!(cycle_ctx.is_cancelled()); } +#[tokio::test] +async fn scanner_cycle_recovery_wake_survives_wait_registration_race() { + notify_scanner_cycle_recovery_wake(); + + tokio::time::timeout(Duration::from_secs(1), SCANNER_CYCLE_RECOVERY_WAKE.notified()) + .await + .expect("recovery wake should retain a permit until the waiter registers"); +} + struct ScannerDefaultSpeedGuard; impl ScannerDefaultSpeedGuard { @@ -151,6 +161,7 @@ impl Drop for ScannerDefaultCycleGuard { struct MemoryConfigStore { objects: Mutex>>, revisions: Mutex>, + non_regular_objects: Mutex>, fail_put_number: Mutex>, object_not_found_put_number: Mutex>, error_after_commit_put_number: Mutex>, @@ -191,12 +202,16 @@ impl crate::storage_api::scanner_io::ObjectIO for MemoryConfigStore { .get(&key) .cloned() .ok_or(EcstoreError::FileNotFound)?; - let revision = *self.revisions.lock().await.entry(key).or_insert(1); + let data_len = i64::try_from(data.len()).expect("memory test object length should fit in i64"); + let revision = *self.revisions.lock().await.entry(key.clone()).or_insert(1); + let is_dir = self.non_regular_objects.lock().await.contains(&key); Ok(GetObjectReader { stream: Box::new(Cursor::new(data)), object_info: ObjectInfo { etag: Some(format!("memory-{revision}")), + size: data_len, + is_dir, ..Default::default() }, buffered_body: None, @@ -797,6 +812,10 @@ fn scanner_cycle_state_decodes_legacy_and_fenced_formats() { let (fenced_cycle, fenced_epoch) = decode_scanner_cycle_state(&fenced).expect("fenced cycle state should decode"); assert_eq!(fenced_cycle.next, 13); assert_eq!(fenced_epoch, 7); + + let mut trailing = fenced; + trailing.push(0); + assert!(decode_scanner_cycle_state(&trailing).is_err()); } #[test] @@ -823,6 +842,840 @@ fn scanner_startup_fails_closed_on_nonempty_corrupt_cycle_state() { assert!(encode_scanner_cycle_state(&exhausted, 7).is_err()); } +#[tokio::test] +async fn corrupt_cycle_state_is_quarantined_once() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + store.objects.lock().await.insert(state_key.clone(), vec![1]); + store.revisions.lock().await.insert(state_key.clone(), 7); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store.clone()).await, + ScannerCycleStateStartup::Blocked + )); + let marker_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + let marker_data = store + .objects + .lock() + .await + .get(&marker_key) + .cloned() + .expect("corrupt state must leave a durable recovery marker"); + let marker: ScannerCycleRecoveryMarker = serde_json::from_slice(&marker_data).expect("marker should be valid JSON"); + assert_eq!(marker.primary_revision, "memory-7"); + assert_eq!(marker.path, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + assert_eq!(marker.quarantine_path, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + assert_eq!(marker.classification, "corrupt"); + + // A second startup sees the matching marker before consuming the poison body. + assert!(matches!( + load_scanner_cycle_state_for_startup(store.clone()).await, + ScannerCycleStateStartup::Blocked + )); + + // Replacing the primary object advances its revision; the stale marker must + // not quarantine the newer, valid state. + let cycle = CurrentCycle { + next: 9, + ..Default::default() + }; + let encoded = encode_scanner_cycle_state(&cycle, 3).expect("valid state should encode"); + store.objects.lock().await.insert(state_key.clone(), encoded); + store.revisions.lock().await.insert(state_key, 8); + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Ready { + cycle: CurrentCycle { next: 9, .. }, + leader_epoch: 3, + .. + } + )); +} + +#[tokio::test] +async fn empty_cycle_state_object_is_quarantined_as_corrupt() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + store.objects.lock().await.insert(state_key.clone(), Vec::new()); + store.revisions.lock().await.insert(state_key, 6); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Blocked + )); + assert_eq!(scanner_cycle_recovery_status().classification.as_deref(), Some("corrupt")); + assert!( + scanner_cycle_recovery_status() + .reason + .as_deref() + .is_some_and(|reason| reason.contains("empty")) + ); +} + +#[tokio::test] +async fn future_cycle_state_schema_is_recovery_required() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + let mut future = 17_u64.to_le_bytes().to_vec(); + future.extend_from_slice(b"RSCYC999"); + future.extend_from_slice(&4_u64.to_le_bytes()); + future.extend_from_slice(&[0x90]); + store.objects.lock().await.insert(state_key.clone(), future); + store.revisions.lock().await.insert(state_key, 13); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Blocked + )); + assert_eq!(scanner_cycle_recovery_status().classification.as_deref(), Some("future_schema")); +} + +#[tokio::test] +async fn concurrent_leaders_cannot_quarantine_newer_cycle_state() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + store.objects.lock().await.insert(state_key.clone(), vec![1]); + store.revisions.lock().await.insert(state_key, 4); + + let (first, second) = tokio::join!( + load_scanner_cycle_state_for_startup(store.clone()), + load_scanner_cycle_state_for_startup(store.clone()), + ); + assert!(matches!(first, ScannerCycleStateStartup::Blocked)); + assert!(matches!(second, ScannerCycleStateStartup::Blocked)); + + let marker_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + let marker_data = store + .objects + .lock() + .await + .get(&marker_key) + .cloned() + .expect("one contender must publish the recovery marker"); + let marker: ScannerCycleRecoveryMarker = serde_json::from_slice(&marker_data).expect("marker should decode"); + assert_eq!(marker.primary_revision, "memory-4"); +} + +#[tokio::test] +async fn cleanup_pending_marker_blocks_a_rewritten_primary_after_restart() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + let marker_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + let encoded = encode_scanner_cycle_state( + &CurrentCycle { + next: 12, + ..Default::default() + }, + 8, + ) + .expect("valid state should encode"); + store.objects.lock().await.insert(state_key.clone(), encoded); + store.revisions.lock().await.insert(state_key, 22); + let marker = ScannerCycleRecoveryMarker { + schema_version: 1, + primary_revision: "memory-21".to_string(), + generation: 11, + leader_epoch: 7, + classification: "corrupt".to_string(), + first_detected_at_unix_secs: 1, + last_attempt_at_unix_secs: 2, + retry_count: 1, + reason: "reset in progress".to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "cleanup-pending".to_string(), + }; + store + .objects + .lock() + .await + .insert(marker_key.clone(), serde_json::to_vec(&marker).expect("marker should encode")); + store.revisions.lock().await.insert(marker_key, 3); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Blocked + )); + assert_eq!(scanner_cycle_recovery_status().state, "cleanup-pending"); +} + +#[test] +fn full_rescan_reset_accepts_unknown_marker_fields_without_trusting_cursor() { + let marker = br#"{ + "schema_version": 99, + "primary_revision": "memory-7", + "generation": 9000, + "leader_epoch": 9000, + "classification": "new-future-classification", + "first_detected_at_unix_secs": 1, + "last_attempt_at_unix_secs": 2, + "retry_count": 9, + "reason": "future marker", + "path": "buckets/.bloomcycle.bin", + "quarantine_path": "buckets/.bloomcycle.bin.recovery-required.json", + "future_field": {"cursor": "untrusted"} + }"#; + let decoded = + super::cycle_state::decode_recovery_marker_for_reset(marker, &DataUsageCacheRevision::Etag("memory-3".to_string())) + .expect("full-rescan compatibility decoder should accept additive fields"); + assert_eq!(decoded.primary_revision, "memory-7"); + assert_eq!(decoded.classification, "future_schema"); + assert_eq!(decoded.generation, 0); + assert_eq!(decoded.leader_epoch, 0); + assert_eq!(decoded.state, "blocked"); + + let malformed = + super::cycle_state::decode_recovery_marker_for_reset(b"{not-json", &DataUsageCacheRevision::Etag("memory-4".to_string())) + .expect("a full-rescan reset must recover even when the marker is malformed"); + assert!(malformed.primary_revision.is_empty()); + assert_eq!(malformed.classification, "future_schema"); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_after_malformed_marker_without_trusting_cursor() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0xff, 0x00, 0x01]) + .await + .expect("corrupt cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), br#"{not-json"#.to_vec()) + .await + .expect("malformed marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("full-rescan reset should recover malformed marker"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(cycle.next, 0, "reset must use the verified usage floor, not marker cursor"); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_ignores_epoch_from_malformed_future_primary() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let mut future_primary = vec![0; 24]; + future_primary[8..16].copy_from_slice(b"RSCY9999"); + future_primary[16..24].copy_from_slice(&u64::MAX.to_le_bytes()); + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), future_primary) + .await + .expect("future cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), br#"{not-json"#.to_vec()) + .await + .expect("malformed marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("full-rescan reset should recover malformed future state"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (_, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(leader_epoch, 1, "invalid persisted bytes must not raise the recovery epoch"); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn ecstore_exact_recovery_marker_delete_honors_etag() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"marker-v1".to_vec()) + .await + .expect("initial recovery marker should be persisted"); + let (_, stale_revision) = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("initial marker revision should load"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"marker-v2".to_vec()) + .await + .expect("replacement recovery marker should be persisted"); + + let delete_result = store + .delete_config_object( + RUSTFS_META_BUCKET, + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + ObjectOptions { + http_preconditions: Some(stale_revision.preconditions()), + ..Default::default() + }, + ) + .await; + assert!(matches!(delete_result, Err(EcstoreError::PreconditionFailed))); + assert_eq!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("replacement marker should remain durable"), + b"marker-v2" + ); +} + +#[tokio::test] +async fn full_rescan_reset_rejects_corrupt_primary_under_stale_blocked_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let corrupt_primary = vec![0xff, 0x00, 0x01]; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), corrupt_primary.clone()) + .await + .expect("corrupt cycle state should be persisted"); + let (_, primary_revision) = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary revision should load"); + let marker = ScannerCycleRecoveryMarker { + schema_version: 1, + primary_revision: "memory-stale".to_string(), + generation: 1, + leader_epoch: 1, + classification: "corrupt".to_string(), + first_detected_at_unix_secs: 1, + last_attempt_at_unix_secs: 2, + retry_count: 1, + reason: "blocked primary changed".to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "blocked".to_string(), + }; + let marker_data = serde_json::to_vec(&marker).expect("blocked marker should encode"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), marker_data.clone()) + .await + .expect("blocked marker should be persisted"); + + assert!( + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .is_err(), + "a strict marker must fail closed when its primary revision changed" + ); + assert_eq!( + read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary should remain readable"), + corrupt_primary + ); + assert_eq!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("blocked marker should remain durable"), + marker_data + ); + assert!(!matches!(primary_revision, DataUsageCacheRevision::Missing)); +} + +#[tokio::test] +async fn full_rescan_reset_preserves_valid_primary_when_marker_is_malformed() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let primary = CurrentCycle { + next: 42, + ..Default::default() + }; + let old_primary_data = encode_scanner_cycle_state(&primary, 7).expect("valid cycle state should encode"); + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), old_primary_data.clone()) + .await + .expect("valid cycle state should be persisted"); + let (_, old_primary_revision) = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary state revision should load"); + let old_usage = DataUsageInfo { + scanner_epoch: Some(7), + scanner_cycle: Some(41), + ..Default::default() + }; + let old_usage_data = serde_json::to_vec(&old_usage).expect("usage snapshot should encode"); + save_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), old_usage_data.clone()) + .await + .expect("usage snapshot should be persisted"); + let (_, old_usage_revision) = read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("usage snapshot revision should load"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("reset should clear a stale malformed marker"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("valid primary should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("primary cycle state should decode"); + assert_eq!(cycle.next, 42, "reset must not regress an independently fenced primary"); + assert_eq!(leader_epoch, 8, "reset must advance the preserved primary epoch"); + let stale_primary_save = save_config_with_preconditions( + store.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + old_primary_data, + old_primary_revision.preconditions(), + ) + .await; + assert!(matches!(stale_primary_save, Err(EcstoreError::PreconditionFailed))); + let usage = read_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("usage epoch fence should remain durable"); + assert_eq!( + serde_json::from_slice::(&usage) + .expect("fenced usage should decode") + .scanner_epoch, + Some(8) + ); + let stale_save = save_config_with_preconditions( + store.clone(), + DATA_USAGE_OBJ_NAME_PATH.as_str(), + old_usage_data, + old_usage_revision.preconditions(), + ) + .await; + assert!(matches!(stale_save, Err(EcstoreError::PreconditionFailed))); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_resumes_cleanup_pending_preserved_primary() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let completed_at = Utc::now(); + let primary = CurrentCycle { + current: 3, + next: 42, + cycle_completed: vec![completed_at], + started: completed_at, + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + encode_scanner_cycle_state(&primary, 7).expect("valid cycle state should encode"), + ) + .await + .expect("valid cycle state should be persisted"); + let usage = DataUsageInfo { + scanner_epoch: Some(7), + scanner_cycle: Some(41), + ..Default::default() + }; + save_config( + store.clone(), + DATA_USAGE_OBJ_NAME_PATH.as_str(), + serde_json::to_vec(&usage).expect("usage snapshot should encode"), + ) + .await + .expect("usage snapshot should be persisted"); + let marker = ScannerCycleRecoveryMarker { + schema_version: 1, + primary_revision: "memory-old".to_string(), + generation: 41, + leader_epoch: 7, + classification: "corrupt".to_string(), + first_detected_at_unix_secs: 1, + last_attempt_at_unix_secs: 2, + retry_count: 1, + reason: "reset in progress".to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "cleanup-pending".to_string(), + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + serde_json::to_vec(&marker).expect("marker should encode"), + ) + .await + .expect("cleanup marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("reset should resume a cleanup-pending preserved primary"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("preserved cycle state should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("cycle state should decode"); + assert_eq!(cycle.current, 3, "cleanup retry must preserve the in-progress cursor"); + assert_eq!(cycle.next, 42); + assert_eq!(cycle.cycle_completed, vec![completed_at]); + assert_eq!(cycle.started, completed_at); + assert_eq!(leader_epoch, 8); + let usage = read_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("usage epoch fence should remain durable"); + assert_eq!( + serde_json::from_slice::(&usage) + .expect("usage should decode") + .scanner_epoch, + Some(8) + ); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_oversized_regular_primary_with_malformed_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0; 1024 * 1024 + 1]) + .await + .expect("oversized cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("explicit full-rescan reset should replace an oversized regular primary"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(cycle.next, 0); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_oversized_primary_after_cleanup_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0; 1024 * 1024 + 1]) + .await + .expect("oversized cycle state should be persisted"); + let (_, primary_revision) = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary revision should load"); + let marker = ScannerCycleRecoveryMarker { + schema_version: 1, + primary_revision: match primary_revision { + DataUsageCacheRevision::Etag(etag) => etag, + DataUsageCacheRevision::Missing => panic!("primary revision should be present"), + }, + generation: 1, + leader_epoch: 1, + classification: "corrupt".to_string(), + first_detected_at_unix_secs: 1, + last_attempt_at_unix_secs: 2, + retry_count: 1, + reason: "reset in progress".to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "cleanup-pending".to_string(), + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + serde_json::to_vec(&marker).expect("cleanup marker should encode"), + ) + .await + .expect("cleanup marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("cleanup retry should rebuild an oversized primary"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(cycle.next, 0); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_with_oversized_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0xff, 0x00, 0x01]) + .await + .expect("corrupt cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), vec![b'x'; 64 * 1024 + 1]) + .await + .expect("oversized recovery marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("full-rescan reset should recover an oversized marker"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (_, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_with_empty_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0xff, 0x00, 0x01]) + .await + .expect("corrupt cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), Vec::new()) + .await + .expect("empty recovery marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("full-rescan reset should recover an empty marker"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (_, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_keeps_cleanup_marker_when_preserved_epoch_is_exhausted() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let primary = CurrentCycle { + next: 42, + ..Default::default() + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + encode_scanner_cycle_state(&primary, u64::MAX).expect("valid cycle state should encode"), + ) + .await + .expect("valid cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + assert!( + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .is_err() + ); + + let marker = read_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("cleanup marker should remain durable"); + assert_eq!( + serde_json::from_slice::(&marker) + .expect("cleanup marker should decode") + .state, + "cleanup-pending" + ); + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Blocked + )); +} + +#[tokio::test] +async fn full_rescan_reset_rejects_preserved_epoch_that_would_be_terminal() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let primary = CurrentCycle { + next: 42, + ..Default::default() + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + encode_scanner_cycle_state(&primary, u64::MAX - 1).expect("valid cycle state should encode"), + ) + .await + .expect("valid cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + assert!( + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .is_err(), + "reset must not persist the terminal leader epoch" + ); + + let marker = read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("cleanup marker should remain durable"); + assert_eq!( + serde_json::from_slice::(&marker) + .expect("cleanup marker should decode") + .state, + "cleanup-pending" + ); +} + +#[tokio::test] +async fn full_rescan_reset_rejects_usage_floor_that_would_be_terminal() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), vec![0xff, 0x00, 0x01]) + .await + .expect("corrupt cycle state should be persisted"); + save_config( + store.clone(), + DATA_USAGE_OBJ_NAME_PATH.as_str(), + serde_json::to_vec(&DataUsageInfo { + scanner_epoch: Some(u64::MAX - 1), + ..Default::default() + }) + .expect("usage floor should encode"), + ) + .await + .expect("usage floor should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + assert!( + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .is_err(), + "reset must not persist the terminal leader epoch" + ); + assert_eq!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("recovery marker should remain durable"), + b"{not-json" + ); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_empty_primary_with_malformed_marker() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), Vec::new()) + .await + .expect("empty cycle state should be persisted"); + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{not-json".to_vec()) + .await + .expect("malformed marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("explicit full-rescan reset should replace an empty primary"); + + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt cycle state should remain durable"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(cycle.next, 0); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn full_rescan_reset_rebuilds_when_primary_cycle_state_is_missing() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + let marker = ScannerCycleRecoveryMarker { + schema_version: 1, + primary_revision: "memory-missing".to_string(), + generation: u64::MAX, + leader_epoch: u64::MAX, + classification: "corrupt".to_string(), + first_detected_at_unix_secs: 1, + last_attempt_at_unix_secs: 2, + retry_count: 0, + reason: "missing primary".to_string(), + path: DATA_USAGE_BLOOM_NAME_PATH.clone(), + quarantine_path: DATA_USAGE_BLOOM_RECOVERY_PATH.clone(), + state: "blocked".to_string(), + }; + save_config( + store.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + serde_json::to_vec(&marker).expect("marker should encode"), + ) + .await + .expect("marker should be persisted"); + + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("full-rescan reset should recreate missing primary"); + let state = read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("missing primary should be rebuilt"); + let (cycle, leader_epoch) = decode_scanner_cycle_state(&state).expect("rebuilt cycle state should decode"); + assert_eq!(cycle.next, 0); + assert_eq!(leader_epoch, 1); + assert!(matches!( + read_config(store, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); +} + +#[tokio::test] +async fn corrupt_cycle_state_rename_or_marker_failure_stays_recovery_required() { + let store = Arc::new(MemoryConfigStore::default()); + let state_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + let marker_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + store.objects.lock().await.insert(state_key.clone(), vec![1]); + store.revisions.lock().await.insert(state_key, 9); + store.fail_put_number.lock().await.insert(marker_key, 1); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store.clone()).await, + ScannerCycleStateStartup::Transient(_) + )); + let status = scanner_cycle_recovery_status(); + assert_eq!(status.state, "recovery-required"); + assert!(status.retryable); + assert!( + store + .objects + .lock() + .await + .contains_key(&memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str())) + ); +} + +#[tokio::test] +async fn oversized_or_symlinked_cycle_state_is_rejected() { + let store = Arc::new(MemoryConfigStore::default()); + let key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_NAME_PATH.as_str()); + store.objects.lock().await.insert(key.clone(), vec![0; 1024 * 1024 + 1]); + store.revisions.lock().await.insert(key.clone(), 11); + + assert!(matches!( + load_scanner_cycle_state_for_startup(store.clone()).await, + ScannerCycleStateStartup::Blocked + )); + assert_eq!(scanner_cycle_recovery_status().classification.as_deref(), Some("corrupt")); + assert!( + scanner_cycle_recovery_status() + .reason + .as_deref() + .is_some_and(|reason| reason.contains("oversized")) + ); + + let marker_key = memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()); + store.objects.lock().await.remove(&marker_key); + store.objects.lock().await.insert(key.clone(), vec![1]); + store.revisions.lock().await.insert(key.clone(), 12); + store.non_regular_objects.lock().await.insert(key); + // The object contract exposes a non-regular object as `is_dir`; local + // backends reject symlink/reparse entries before they become an object. + assert!(matches!( + load_scanner_cycle_state_for_startup(store).await, + ScannerCycleStateStartup::Blocked + )); +} + #[tokio::test] async fn scanner_startup_uses_primary_and_backup_usage_floor() { let store = Arc::new(MemoryConfigStore::default()); @@ -855,6 +1708,31 @@ async fn scanner_startup_uses_primary_and_backup_usage_floor() { assert_eq!(epoch, 11); } +#[tokio::test] +async fn scanner_usage_floor_ignores_older_backup_after_primary_epoch_fence() { + let store = Arc::new(MemoryConfigStore::default()); + let backup_path = format!("{}.bkp", DATA_USAGE_OBJ_NAME_PATH.as_str()); + for (path, epoch, cycle) in [(DATA_USAGE_OBJ_NAME_PATH.as_str(), 8, 100), (backup_path.as_str(), 7, 10_000)] { + store.objects.lock().await.insert( + memory_config_key(RUSTFS_META_BUCKET, path), + serde_json::to_vec(&DataUsageInfo { + scanner_epoch: Some(epoch), + scanner_cycle: Some(cycle), + ..Default::default() + }) + .expect("usage snapshot should encode"), + ); + } + + assert_eq!( + persisted_usage_floor(store).await.expect("usage floor should load"), + PersistedUsageFloor { + next_cycle: 101, + leader_epoch: 8, + } + ); +} + #[test] fn scanner_startup_treats_incomplete_usage_snapshot_as_cold() { let mut legacy = complete_usage_with_bucket_count(Some(std::time::SystemTime::now()), 1); @@ -987,6 +1865,15 @@ async fn scanner_usage_floor_fails_closed_on_corrupt_or_exhausted_usage_state() assert!(persisted_usage_floor(store.clone()).await.is_err()); + store.objects.lock().await.insert( + memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_OBJ_NAME_PATH.as_str()), + br#"{}"#.to_vec(), + ); + assert!( + persisted_usage_floor(store.clone()).await.is_err(), + "a structurally incomplete usage snapshot must not be treated as an empty floor" + ); + store.objects.lock().await.insert( memory_config_key(RUSTFS_META_BUCKET, DATA_USAGE_OBJ_NAME_PATH.as_str()), serde_json::to_vec(&DataUsageInfo { @@ -1244,6 +2131,22 @@ async fn test_leadership_claim_preserves_usage_epoch_floor_across_old_epoch_conf assert_eq!(store.put_counts.lock().await.get(&key), Some(&3)); } +#[tokio::test] +async fn test_leadership_claim_rejects_terminal_epoch() { + let store = Arc::new(MemoryConfigStore::default()); + let ctx = CancellationToken::new(); + let mut revision = DataUsageCacheRevision::Missing; + let mut cycle = CurrentCycle { + next: 12, + ..Default::default() + }; + let mut persisted_epoch = u64::MAX - 1; + + assert!(!claim_scanner_leadership(&ctx, store.clone(), &mut cycle, &mut revision, &mut persisted_epoch).await); + assert_eq!(persisted_epoch, u64::MAX - 1); + assert!(read_config(store, &DATA_USAGE_BLOOM_NAME_PATH).await.is_err()); +} + #[tokio::test] async fn test_leadership_claim_confirms_commit_after_returned_error() { let store = Arc::new(MemoryConfigStore::default()); @@ -2975,6 +3878,24 @@ fn superseded_retry_backoff_grows_from_the_default_cycle() { } } +#[tokio::test(start_paused = true)] +async fn corrupt_cycle_state_backoff_uses_virtual_clock() { + let mut backoff = ScannerRetryBackoff::default(); + backoff.record_retryable_cycle(true); + let first_delay = backoff + .retry_interval(Duration::from_secs(60)) + .expect("the first recovery retry should be scheduled"); + assert_eq!(first_delay, Duration::from_secs(5)); + + let deadline = Instant::now() + first_delay; + assert!(Instant::now() < deadline); + tokio::time::advance(first_delay).await; + assert!(Instant::now() >= deadline); + + backoff.record_retryable_cycle(true); + assert_eq!(backoff.retry_interval(Duration::from_secs(60)), Some(Duration::from_secs(10))); +} + #[test] fn scanner_cycle_wait_plan_drives_growth_resets_and_bitrot_cap() { let runtime_config = ScannerRuntimeConfig { diff --git a/rustfs/src/admin/handlers/mod.rs b/rustfs/src/admin/handlers/mod.rs index f0a32f402..6822ccaa4 100644 --- a/rustfs/src/admin/handlers/mod.rs +++ b/rustfs/src/admin/handlers/mod.rs @@ -126,6 +126,7 @@ mod tests { let _list_remote_target_handler = replication::ListRemoteTargetHandler {}; let _remove_remote_target_handler = replication::RemoveRemoteTargetHandler {}; let _scanner_status_handler = scanner::ScannerStatusHandler {}; + let _scanner_cycle_state_reset_handler = scanner::ScannerCycleStateResetHandler {}; let _ilm_expiry_status_handler = scanner::IlmExpiryStatusHandler {}; let _manual_transition_handler = ilm_transition::ManualTransitionRunHandler {}; let _manual_transition_status_handler = ilm_transition::ManualTransitionJobStatusHandler {}; diff --git a/rustfs/src/admin/handlers/scanner.rs b/rustfs/src/admin/handlers/scanner.rs index ad500004d..fa8df2a69 100644 --- a/rustfs/src/admin/handlers/scanner.rs +++ b/rustfs/src/admin/handlers/scanner.rs @@ -13,8 +13,11 @@ // limitations under the License. use crate::admin::auth::authorize_admin_request; +use crate::admin::handlers::supervise_admin_mutation; use crate::admin::router::{AdminOperation, Operation, S3Router}; -use crate::admin::runtime_sources::current_scanner_metrics_report; +use crate::admin::runtime_sources::{ + app_context_from_req, current_object_store_handle_for_context, current_scanner_metrics_report, +}; use crate::module_switches::{ENV_SCANNER_ENABLED, scanner_enabled_from_env}; use crate::server::ADMIN_PREFIX; use chrono::Utc; @@ -22,11 +25,13 @@ use http::{HeaderMap, HeaderValue}; use hyper::{Method, StatusCode}; use matchit::Params; use rustfs_common::metrics::{ScannerLifecycleExpirySnapshot, ScannerMaintenanceControlSnapshot, ScannerMetricsReport}; +use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE; use rustfs_credentials::Credentials; use rustfs_policy::policy::action::{Action, AdminAction}; use s3s::header::CONTENT_TYPE; use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, s3_error}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; +use tokio_util::sync::CancellationToken; const JSON_CONTENT_TYPE: &str = "application/json"; @@ -38,6 +43,13 @@ struct ScannerStatusResponse { metrics: ScannerMetricsReport, cycle_schedule: rustfs_scanner::ScannerCycleScheduleStatus, runtime_config: rustfs_scanner::runtime_config::ScannerRuntimeConfigStatus, + cycle_recovery: rustfs_scanner::ScannerCycleRecoveryStatus, +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct ScannerCycleResetRequest { + mode: String, } #[derive(Debug, Serialize)] @@ -117,6 +129,7 @@ fn scanner_status_response( metrics, cycle_schedule, runtime_config, + cycle_recovery: rustfs_scanner::scanner::scanner_cycle_recovery_status(), } } @@ -144,6 +157,11 @@ pub fn register_scanner_route(r: &mut S3Router) -> std::io::Resu format!("{ADMIN_PREFIX}/v3/scanner/status").as_str(), AdminOperation(&ScannerStatusHandler {}), )?; + r.insert( + Method::POST, + format!("{ADMIN_PREFIX}/v3/scanner/cycle-state/reset").as_str(), + AdminOperation(&ScannerCycleStateResetHandler {}), + )?; r.insert( Method::GET, format!("{ADMIN_PREFIX}/v3/ilm/expiry/status").as_str(), @@ -163,6 +181,13 @@ async fn validate_scanner_status_request(req: &S3Request) -> S3Result) -> S3Result { + if req.credentials.is_none() { + return Err(s3_error!(InvalidRequest, "missing credentials")); + } + authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ConfigUpdateAdminAction)]).await +} + fn json_response(body: Vec) -> S3Result> { let mut headers = HeaderMap::new(); let content_type = HeaderValue::from_str(JSON_CONTENT_TYPE) @@ -192,6 +217,37 @@ impl Operation for ScannerStatusHandler { pub struct IlmExpiryStatusHandler {} +pub struct ScannerCycleStateResetHandler {} + +#[async_trait::async_trait] +impl Operation for ScannerCycleStateResetHandler { + async fn call(&self, mut req: S3Request, _params: Params<'_, '_>) -> S3Result> { + let _cred = validate_scanner_reset_request(&req).await?; + let body = req + .input + .store_all_limited(MAX_ADMIN_REQUEST_BODY_SIZE) + .await + .map_err(|err| S3Error::with_message(S3ErrorCode::InvalidRequest, format!("invalid reset request body: {err}")))?; + let reset = serde_json::from_slice::(&body) + .map_err(|err| S3Error::with_message(S3ErrorCode::InvalidRequest, format!("invalid reset request body: {err}")))?; + if reset.mode != "full-rescan" { + return Err(S3Error::with_message(S3ErrorCode::InvalidRequest, "reset mode must be full-rescan")); + } + let context = app_context_from_req(&req) + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "storage layer not initialized"))?; + let store = current_object_store_handle_for_context(Some(context.as_ref())) + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "storage layer not initialized"))?; + supervise_admin_mutation("scanner cycle state reset", async move { + rustfs_scanner::scanner::reset_scanner_cycle_recovery(CancellationToken::new(), store) + .await + .map_err(|err| S3Error::with_message(S3ErrorCode::InternalError, err.to_string()))?; + Ok::<_, S3Error>(()) + }) + .await?; + json_response(br#"{"status":"reset","mode":"full-rescan"}"#.to_vec()) + } +} + #[async_trait::async_trait] impl Operation for IlmExpiryStatusHandler { async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { @@ -237,6 +293,38 @@ mod tests { assert_eq!(err.message(), Some("missing credentials")); } + #[tokio::test] + async fn scanner_reset_gate_rejects_missing_credentials() { + let req = S3Request { + input: Body::from(String::new()), + method: Method::POST, + uri: http::Uri::from_static("/rustfs/admin/v3/scanner/cycle-state/reset"), + headers: HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = validate_scanner_reset_request(&req) + .await + .expect_err("a reset request without credentials must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("missing credentials")); + } + + #[test] + fn admin_reset_requires_full_rescan_or_verified_cursor() { + let full_rescan: ScannerCycleResetRequest = + serde_json::from_str(r#"{"mode":"full-rescan"}"#).expect("full rescan must be accepted"); + assert_eq!(full_rescan.mode, "full-rescan"); + let cursor: ScannerCycleResetRequest = + serde_json::from_str(r#"{"mode":"cursor"}"#).expect("mode validation belongs to the handler"); + assert_ne!(cursor.mode, "full-rescan"); + assert!(serde_json::from_str::(r#"{"mode":"full-rescan","cursor":"untrusted"}"#).is_err()); + } + #[test] fn scanner_disabled_reason_reports_startup_env_key() { assert_eq!(scanner_disabled_reason(true), None); @@ -304,6 +392,11 @@ mod tests { assert_eq!(encoded["cycle_schedule"]["effective_interval_seconds"], 0); assert_eq!(encoded["cycle_schedule"]["clean_idle_backoff_enabled"], false); assert_eq!(encoded["cycle_schedule"]["clean_idle_backoff_multiplier"], 1); + assert_eq!(encoded["cycle_recovery"]["state"], "healthy"); + assert_eq!( + encoded["cycle_recovery"]["quarantine_path"], + rustfs_scanner::DATA_USAGE_BLOOM_RECOVERY_PATH.as_str() + ); } #[test] diff --git a/rustfs/src/admin/route_policy.rs b/rustfs/src/admin/route_policy.rs index ccb013211..e423c0b0e 100644 --- a/rustfs/src/admin/route_policy.rs +++ b/rustfs/src/admin/route_policy.rs @@ -428,6 +428,12 @@ pub const ADMIN_ROUTE_POLICY_SPECS: &[AdminRouteSpec] = &[ admin(HttpMethod::Get, "/rustfs/admin/v3/config", CONFIG_UPDATE, RouteRiskLevel::High), admin(HttpMethod::Put, "/rustfs/admin/v3/config", CONFIG_UPDATE, RouteRiskLevel::High), admin(HttpMethod::Get, "/rustfs/admin/v3/scanner/status", SERVER_INFO, RouteRiskLevel::Sensitive), + admin( + HttpMethod::Post, + "/rustfs/admin/v3/scanner/cycle-state/reset", + CONFIG_UPDATE, + RouteRiskLevel::High, + ), admin( HttpMethod::Get, "/rustfs/admin/v3/ilm/expiry/status", @@ -2020,6 +2026,12 @@ mod tests { assert_not_action(HttpMethod::Get, "/rustfs/admin/v3/ilm/expiry/status", SET_TIER); } + #[test] + fn route_policy_requires_config_update_for_scanner_cycle_reset() { + assert_action(HttpMethod::Post, "/rustfs/admin/v3/scanner/cycle-state/reset", CONFIG_UPDATE); + assert_not_action(HttpMethod::Post, "/rustfs/admin/v3/scanner/cycle-state/reset", SERVER_INFO); + } + #[test] fn route_policy_uses_tier_actions_for_transition_routes() { assert_action(HttpMethod::Post, "/rustfs/admin/v3/ilm/transition/run", SET_TIER); diff --git a/rustfs/src/admin/route_registration_test.rs b/rustfs/src/admin/route_registration_test.rs index e48e09c94..8dcb5d901 100644 --- a/rustfs/src/admin/route_registration_test.rs +++ b/rustfs/src/admin/route_registration_test.rs @@ -243,6 +243,7 @@ fn expected_admin_route_matrix() -> Vec { admin_route(Method::GET, "/v3/config"), admin_route(Method::PUT, "/v3/config"), admin_route(Method::GET, "/v3/scanner/status"), + admin_route(Method::POST, "/v3/scanner/cycle-state/reset"), admin_route(Method::GET, "/v3/audit/target/list"), admin_route_sample( Method::PUT, @@ -879,6 +880,7 @@ fn test_register_routes_cover_representative_admin_paths() { assert_route(&router, Method::GET, &admin_path("/v3/config")); assert_route(&router, Method::PUT, &admin_path("/v3/config")); assert_route(&router, Method::GET, &admin_path("/v3/scanner/status")); + assert_route(&router, Method::POST, &admin_path("/v3/scanner/cycle-state/reset")); assert_route(&router, Method::GET, &admin_path("/v3/ilm/expiry/status")); assert_route(&router, Method::POST, &admin_path("/v3/ilm/transition/run")); assert_route( @@ -1367,6 +1369,7 @@ fn test_admin_alias_paths_match_existing_admin_routes() { (Method::GET, compat_admin_alias_path("/v3/config")), (Method::PUT, compat_admin_alias_path("/v3/config")), (Method::GET, compat_admin_alias_path("/v3/scanner/status")), + (Method::POST, compat_admin_alias_path("/v3/scanner/cycle-state/reset")), (Method::GET, compat_admin_alias_path("/v3/ilm/expiry/status")), ] { assert!( From f9d45e41e159dd42de70137ad4ccf37abd525cbb Mon Sep 17 00:00:00 2001 From: cxymds Date: Sat, 22 Aug 2026 19:39:01 +0800 Subject: [PATCH 2/8] fix(quota): account compressed deletes by committed size (#6365) --- crates/ecstore/src/api/mod.rs | 4 +- .../bucket/replication/replication_pool.rs | 2 +- .../replication/replication_resyncer.rs | 2 +- .../replication_target_boundary.rs | 16 +- crates/ecstore/src/core/sets.rs | 123 ++++---- crates/ecstore/src/data_usage/mod.rs | 116 +++++++- crates/ecstore/src/object_api/types.rs | 38 ++- crates/ecstore/src/set_disk/mod.rs | 2 +- crates/ecstore/src/set_disk/ops/object.rs | 163 ++++++++++- .../ecstore/src/storage_api_contracts/mod.rs | 4 +- crates/ecstore/src/store/object.rs | 65 ++++- crates/storage-api/src/lib.rs | 1 + crates/storage-api/src/object.rs | 24 ++ rustfs/src/app/bucket_usecase.rs | 2 +- rustfs/src/app/object_usecase.rs | 271 +++++++++++++++--- rustfs/src/app/storage_api.rs | 10 +- rustfs/src/storage/s3_api/bucket.rs | 44 ++- rustfs/src/storage/storage_api.rs | 2 + 18 files changed, 754 insertions(+), 135 deletions(-) diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index 17fffac3d..456d38ac7 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -317,8 +317,6 @@ pub mod config { } pub mod data_usage { - #[cfg(feature = "test-util")] - pub use crate::data_usage::seed_bucket_usage_memory_for_test; pub use crate::data_usage::{ DATA_USAGE_CACHE_NAME, apply_bucket_usage_memory_overlay, compute_bucket_usage, init_compression_total_memory_from_backend, invalidate_admin_data_usage_snapshot_cache, @@ -330,6 +328,8 @@ pub mod data_usage { remove_bucket_usage_from_backend, replace_bucket_usage_memory_from_info, store_compression_total_in_backend, store_data_usage_in_backend, }; + #[cfg(feature = "test-util")] + pub use crate::data_usage::{get_bucket_usage_memory, seed_bucket_usage_memory_for_test}; } pub mod disk { diff --git a/crates/ecstore/src/bucket/replication/replication_pool.rs b/crates/ecstore/src/bucket/replication/replication_pool.rs index efe517b99..61dbf2f1e 100644 --- a/crates/ecstore/src/bucket/replication/replication_pool.rs +++ b/crates/ecstore/src/bucket/replication/replication_pool.rs @@ -2855,7 +2855,7 @@ fn replicate_object_info_from_object_info( .map(|v| OffsetDateTime::parse(&v, &Rfc3339).unwrap_or(OffsetDateTime::UNIX_EPOCH)); let mut rstate = oi.replication_state(); rstate.replicate_decision_str = dsc.to_string(); - let asz = oi.get_actual_size().unwrap_or_default(); + let asz = oi.get_actual_size_or_physical(); let ssec = replication_object_is_ssec_encrypted(&oi.user_defined); let checksum = if ssec { oi.checksum.clone() } else { None }; diff --git a/crates/ecstore/src/bucket/replication/replication_resyncer.rs b/crates/ecstore/src/bucket/replication/replication_resyncer.rs index d921ea411..e670a7d6f 100644 --- a/crates/ecstore/src/bucket/replication/replication_resyncer.rs +++ b/crates/ecstore/src/bucket/replication/replication_resyncer.rs @@ -1412,7 +1412,7 @@ pub async fn get_heal_replicate_object_info(oi: &ObjectInfo, rcfg: &ReplicationC }; let mut replication_state = oi.replication_state(); replication_state.replicate_decision_str = dsc.to_string(); - let actual_size = oi.get_actual_size().unwrap_or_default(); + let actual_size = oi.get_actual_size_or_physical(); Ok(ReplicateObjectInfo { name: oi.name.clone(), diff --git a/crates/ecstore/src/bucket/replication/replication_target_boundary.rs b/crates/ecstore/src/bucket/replication/replication_target_boundary.rs index 455a37ed0..4fe89967d 100644 --- a/crates/ecstore/src/bucket/replication/replication_target_boundary.rs +++ b/crates/ecstore/src/bucket/replication/replication_target_boundary.rs @@ -389,7 +389,7 @@ fn replication_source_object(object_info: &ObjectInfo) -> ReplicationSourceObjec .map(|mod_time| OffsetDateTime::from_unix_timestamp(mod_time.unix_timestamp()).unwrap_or(mod_time)), version_id: object_info.version_id.map(|version_id| version_id.to_string()), etag: object_info.etag.as_deref(), - actual_size: object_info.get_actual_size().unwrap_or_default(), + actual_size: object_info.get_actual_size_or_physical(), delete_marker: object_info.delete_marker, content_type: object_info.content_type.as_deref(), content_encoding: object_info.content_encoding.as_deref(), @@ -542,6 +542,20 @@ mod tests { assert!(replication_target_head_is_newer_null_version(&source, &target)); } + #[test] + fn replication_source_uses_physical_size_for_unknown_compressed_object() { + let mut metadata = HashMap::new(); + rustfs_utils::http::insert_str(&mut metadata, rustfs_utils::http::SUFFIX_COMPRESSION, "zstd".to_string()); + let source = ObjectInfo { + size: 128, + actual_size: -1, + user_defined: Arc::new(metadata), + ..Default::default() + }; + + assert_eq!(replication_source_object(&source).actual_size, 128); + } + #[test] fn replication_target_head_content_matches_compare_etag_only() { let source = ObjectInfo { diff --git a/crates/ecstore/src/core/sets.rs b/crates/ecstore/src/core/sets.rs index 3acf1a705..d9b354a08 100644 --- a/crates/ecstore/src/core/sets.rs +++ b/crates/ecstore/src/core/sets.rs @@ -21,7 +21,7 @@ use crate::storage_api_contracts::{ bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOptions, MakeBucketOptions}, list::{StorageListObjectVersionsInfo, StorageListObjectsV2Info, StorageObjectInfoOrErr, StorageWalkOptions}, multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo}, - object::{DeletedObject, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, + object::{DeleteAccounting, DeletedObject, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, range::HTTPRangeSpec, }; use crate::{ @@ -414,6 +414,66 @@ fn apply_delete_objects_results( } } +fn apply_delete_accounting_results( + accounting: &mut [Option], + set_objects: &[DelObj], + set_accounting: &[Option], +) { + for (obj, value) in set_objects.iter().zip(set_accounting.iter()) { + accounting[obj.orig_idx] = value.clone(); + } +} + +impl Sets { + pub(crate) async fn delete_objects_with_accounting( + &self, + bucket: &str, + objects: Vec, + opts: ObjectOptions, + ) -> (Vec, Vec>, Vec>) { + let mut del_objects = vec![DeletedObject::default(); objects.len()]; + let mut del_errs = vec![None; objects.len()]; + let mut accounting = vec![None; objects.len()]; + let mut set_obj_map = HashMap::new(); + + for (i, obj) in objects.iter().enumerate() { + let idx = self.get_hashed_set_index(obj.object_name.as_str()); + set_obj_map.entry(idx).or_insert_with(Vec::new).push(DelObj { + orig_idx: i, + obj: obj.clone(), + }); + } + + let max_concurrent = set_obj_map.len().min(num_cpus::get()).max(1); + let semaphore = Arc::new(tokio::sync::Semaphore::new(max_concurrent)); + let mut futures = FuturesUnordered::new(); + let bucket = bucket.to_owned(); + + for (set_index, set_objects) in set_obj_map { + let disks = self.get_disks(set_index); + let objects = set_objects.iter().map(|entry| entry.obj.clone()).collect::>(); + let bucket = bucket.clone(); + let opts = opts.clone(); + let semaphore = semaphore.clone(); + futures.push(async move { + let _permit = semaphore + .acquire_owned() + .await + .expect("delete_objects semaphore should remain open"); + let (deleted, errors, accounting) = disks.delete_objects_with_accounting(&bucket, objects, opts).await; + (set_objects, deleted, errors, accounting) + }); + } + + while let Some((set_objects, deleted, errors, set_accounting)) = futures.next().await { + apply_delete_objects_results(&mut del_objects, &mut del_errs, &set_objects, &deleted, errors); + apply_delete_accounting_results(&mut accounting, &set_objects, &set_accounting); + } + + (del_objects, del_errs, accounting) + } +} + #[async_trait::async_trait] impl crate::storage_api_contracts::object::ObjectIO for Sets { type Error = Error; @@ -655,65 +715,8 @@ impl crate::storage_api_contracts::object::ObjectOperations for Sets { objects: Vec, opts: ObjectOptions, ) -> (Vec, Vec>) { - // Default return value - let mut del_objects = vec![DeletedObject::default(); objects.len()]; - - let mut del_errs = Vec::with_capacity(objects.len()); - for _ in 0..objects.len() { - del_errs.push(None) - } - - let mut set_obj_map = HashMap::new(); - - // hash key - for (i, obj) in objects.iter().enumerate() { - let idx = self.get_hashed_set_index(obj.object_name.as_str()); - - if !set_obj_map.contains_key(&idx) { - set_obj_map.insert( - idx, - vec![DelObj { - // set_idx: idx, - orig_idx: i, - obj: obj.clone(), - }], - ); - } else if let Some(val) = set_obj_map.get_mut(&idx) { - val.push(DelObj { - // set_idx: idx, - orig_idx: i, - obj: obj.clone(), - }); - } - } - - let max_concurrent = set_obj_map.len().min(num_cpus::get()).max(1); - let semaphore = Arc::new(tokio::sync::Semaphore::new(max_concurrent)); - let mut futures = FuturesUnordered::new(); - let bucket = bucket.to_string(); - - for (k, v) in set_obj_map { - let disks = self.get_disks(k); - let objs: Vec = v.iter().map(|v| v.obj.clone()).collect(); - let bucket = bucket.clone(); - let opts = opts.clone(); - let semaphore = semaphore.clone(); - - futures.push(async move { - let _permit = semaphore - .acquire_owned() - .await - .expect("delete_objects semaphore should remain open"); - let (dobjects, errs) = disks.delete_objects(&bucket, objs, opts).await; - (v, dobjects, errs) - }); - } - - while let Some((v, dobjects, errs)) = futures.next().await { - apply_delete_objects_results(&mut del_objects, &mut del_errs, &v, &dobjects, errs); - } - - (del_objects, del_errs) + let (deleted, errors, _) = self.delete_objects_with_accounting(bucket, objects, opts).await; + (deleted, errors) } #[tracing::instrument(skip(self))] diff --git a/crates/ecstore/src/data_usage/mod.rs b/crates/ecstore/src/data_usage/mod.rs index 917edc649..3bf9bf500 100644 --- a/crates/ecstore/src/data_usage/mod.rs +++ b/crates/ecstore/src/data_usage/mod.rs @@ -1391,7 +1391,37 @@ impl BucketUsageAccumulator { } pub fn quota_object_size(object: &ObjectInfo) -> Result { - let logical_size = u64::try_from(object.get_actual_size().map_err(Error::other)?).map_err(|_| Error::PartMissingOrCorrupt)?; + // A compressed object may carry -1 while the transformed size is unknown + // (legacy streaming sentinel). In that case the persisted physical size + // is still a valid accounting floor; every other negative value is corrupt. + // An explicit negative `actual-size` metadata value is corrupt, however: + // the sentinel is only valid in the in-memory/object-part field written by + // the legacy streaming path, not as a persisted declared size. + let compressed = object.is_compressed(); + if object.actual_size < -1 || (object.actual_size == -1 && !compressed) { + return Err(Error::PartMissingOrCorrupt); + } + if object + .parts + .iter() + .any(|part| part.actual_size < -1 || (part.actual_size < 0 && !compressed)) + { + return Err(Error::PartMissingOrCorrupt); + } + let declared_actual_size = rustfs_utils::http::get_str(&object.user_defined, rustfs_utils::http::SUFFIX_ACTUAL_SIZE) + .filter(|value| !value.is_empty()); + if declared_actual_size + .as_deref() + .and_then(|value| value.parse::().ok()) + .is_some_and(|size| size < 0) + { + return Err(Error::PartMissingOrCorrupt); + } + let logical_size = match object.get_actual_size().map_err(Error::other)? { + size if size == -1 && compressed && declared_actual_size.is_none() => None, + size if size >= 0 => Some(u64::try_from(size).map_err(|_| Error::PartMissingOrCorrupt)?), + _ => return Err(Error::PartMissingOrCorrupt), + }; let persisted_part_size = if object.parts.is_empty() { u64::try_from(object.size).map_err(|_| Error::PartMissingOrCorrupt)? } else { @@ -1399,12 +1429,8 @@ pub fn quota_object_size(object: &ObjectInfo) -> Result { // Compressed streaming objects persist -1 when the transformed // part size is unknown. The physical part size remains a valid // quota floor; reject only non-negative values that overflow. - let actual_size = if part.actual_size < 0 { - if object.is_compressed() { - 0 - } else { - return Err(Error::PartMissingOrCorrupt); - } + let actual_size = if part.actual_size == -1 { + 0 } else { u64::try_from(part.actual_size).map_err(|_| Error::PartMissingOrCorrupt)? }; @@ -1412,7 +1438,7 @@ pub fn quota_object_size(object: &ObjectInfo) -> Result { total.checked_add(part_size).ok_or(Error::PartMissingOrCorrupt) })? }; - Ok(logical_size.max(persisted_part_size)) + Ok(logical_size.unwrap_or(0).max(persisted_part_size)) } type UsageVersionPage = StorageListObjectVersionsInfo; @@ -3320,6 +3346,80 @@ mod tests { ); } + #[test] + fn quota_object_size_accepts_compressed_unknown_actual_size_sentinel() { + let mut metadata = HashMap::new(); + rustfs_utils::http::insert_str( + &mut metadata, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + let object = ObjectInfo { + size: 400, + actual_size: -1, + user_defined: Arc::new(metadata), + ..Default::default() + }; + + assert_eq!(quota_object_size(&object).expect("compressed sentinel is valid"), 400); + } + + #[test] + fn quota_object_size_rejects_compressed_part_sum_overflow() { + let mut metadata = HashMap::new(); + rustfs_utils::http::insert_str( + &mut metadata, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + let object = ObjectInfo { + size: 1, + user_defined: Arc::new(metadata), + parts: Arc::new(vec![ + rustfs_filemeta::ObjectPartInfo { + actual_size: i64::MAX, + ..Default::default() + }, + rustfs_filemeta::ObjectPartInfo { + actual_size: 1, + ..Default::default() + }, + ]), + ..Default::default() + }; + + assert!(matches!(quota_object_size(&object), Err(Error::Io(_)))); + } + + #[test] + fn quota_object_size_rejects_negative_values_other_than_the_compressed_sentinel() { + let mut metadata = HashMap::new(); + rustfs_utils::http::insert_str( + &mut metadata, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + let corrupt_object = ObjectInfo { + size: 400, + actual_size: -2, + user_defined: Arc::new(metadata.clone()), + ..Default::default() + }; + assert!(matches!(quota_object_size(&corrupt_object), Err(Error::PartMissingOrCorrupt))); + + let corrupt_part = ObjectInfo { + size: 400, + user_defined: Arc::new(metadata), + parts: Arc::new(vec![rustfs_filemeta::ObjectPartInfo { + size: 400, + actual_size: -2, + ..Default::default() + }]), + ..Default::default() + }; + assert!(matches!(quota_object_size(&corrupt_part), Err(Error::PartMissingOrCorrupt))); + } + #[tokio::test] #[serial] async fn live_bucket_usage_refreshes_are_coalesced_only_while_in_flight() { diff --git a/crates/ecstore/src/object_api/types.rs b/crates/ecstore/src/object_api/types.rs index 1bbff7a7f..0194667e9 100644 --- a/crates/ecstore/src/object_api/types.rs +++ b/crates/ecstore/src/object_api/types.rs @@ -689,6 +689,9 @@ impl ObjectInfo { } pub fn get_actual_size(&self) -> std::io::Result { + if self.actual_size < -1 || (self.actual_size == -1 && !self.is_compressed()) { + return Err(std::io::Error::other("invalid negative actual size")); + } if self.actual_size > 0 { return Ok(self.actual_size); } @@ -700,10 +703,25 @@ impl ObjectInfo { let size = size_str.parse::().map_err(|e| std::io::Error::other(e.to_string()))?; return Ok(size); } - let mut actual_size = 0; - self.parts.iter().for_each(|part| { - actual_size += part.actual_size; - }); + if self.actual_size == -1 && self.parts.is_empty() { + return Ok(-1); + } + let mut actual_size = 0_i64; + let mut unknown = false; + for part in self.parts.iter() { + match part.actual_size { + -1 => unknown = true, + size if size >= 0 => { + actual_size = actual_size + .checked_add(size) + .ok_or_else(|| std::io::Error::other("compressed actual size overflow"))?; + } + _ => return Err(std::io::Error::other("invalid negative compressed part size")), + } + } + if unknown { + return Ok(-1); + } if actual_size == 0 && actual_size != self.size { return Err(std::io::Error::other(format!("invalid decompressed size {} {}", actual_size, self.size))); } @@ -718,6 +736,18 @@ impl ObjectInfo { Ok(self.size) } + /// Returns a non-negative size for client and replication boundaries. + /// + /// Compressed legacy metadata can retain the internal `-1` unknown-size + /// sentinel. Those boundaries cannot emit a negative length, so they use + /// the persisted physical size while quota accounting keeps the sentinel + /// distinction in [`crate::data_usage::quota_object_size`]. + pub fn get_actual_size_or_physical(&self) -> i64 { + self.get_actual_size() + .map(|size| if size >= 0 { size } else { self.size.max(0) }) + .unwrap_or_else(|_| self.size.max(0)) + } + pub fn from_file_info(fi: &FileInfo, bucket: &str, object: &str, versioned: bool) -> ObjectInfo { let mut version_id = fi.version_id; diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index cbf050a4b..e64e1704f 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -97,7 +97,7 @@ use crate::storage_api_contracts::{ CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartOperations as _, MultipartUploadResult, PartInfo, }, namespace::NamespaceLocking as _, - object::{DeletedObject, HTTPPreconditions, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, + object::{DeleteAccounting, DeletedObject, HTTPPreconditions, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, range::HTTPRangeSpec, }; use crate::store::utils::is_reserved_or_invalid_bucket; diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index cb8f70766..faacca3a8 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -45,6 +45,7 @@ use crate::bucket::replication::{ DeleteReplicationConfigSnapshot, ReplicationLifecycleBridge, ReplicationStatusType, VersionPurgeStatusType, replication_state_to_filemeta, replication_status_from_filemeta, version_purge_status_to_filemeta, }; +use crate::data_usage::quota_object_size; use crate::diagnostics::get::GetObjectFailureReason; use crate::disk::{DataDirDeleteStatus, OldCurrentSize}; use crate::error::is_err_invalid_upload_id; @@ -5655,7 +5656,18 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { objects: Vec, opts: ObjectOptions, ) -> (Vec, Vec>) { + let (deleted, errors, _) = self.delete_objects_with_accounting(bucket, objects, opts).await; + (deleted, errors) + } + + async fn delete_objects_with_accounting( + &self, + bucket: &str, + objects: Vec, + opts: ObjectOptions, + ) -> (Vec, Vec>, Vec>) { let mut del_objects = vec![DeletedObject::default(); objects.len()]; + let mut accounting = vec![None; objects.len()]; let delete_config_snapshot = opts .delete_replication_config_snapshot .clone() @@ -5745,7 +5757,7 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { *item = Some(Error::other(message.clone())); } } - return (del_objects, del_errs); + return (del_objects, del_errs, accounting); } }, } @@ -5792,6 +5804,22 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { let source_missing = gerr .as_ref() .is_some_and(|err| is_err_object_not_found(err) || is_err_version_not_found(err)); + // Resolve accounting from the generation selected under this + // object's write lock. A request-layer pre-stat is only an + // optimization and cannot identify a concurrent overwrite. + let (accounting_size, accounting_version_id, removed_current_object) = if source_missing + || dobj.synthetic_version_id + || set_disk_delete_creates_delete_marker(&check_opts) + || goi.delete_marker + { + (None, None, false) + } else { + ( + quota_object_size(&goi).ok(), + goi.version_id.filter(|version_id| !version_id.is_nil()), + (dobj.version_id.is_none() || is_explicit_null_version(dobj.version_id)) && !dobj.synthetic_version_id, + ) + }; // Normalize both sides before comparing. `goi.version_id` is the // client-facing identity, where `from_file_info` synthesizes // `Some(Uuid::nil())` for a null version on a versioned or @@ -5920,7 +5948,12 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { }, replication_state: vr.replication_state_internal.clone(), ..Default::default() - } + }; + accounting[i] = Some(DeleteAccounting { + size: accounting_size, + version_id: accounting_version_id, + removed_current_object, + }); } // Only add to vers_map if we hold the lock @@ -5966,7 +5999,7 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { }); } } - return (del_objects, del_errs); + return (del_objects, del_errs, accounting); } let mut persisted_journal_entries = Vec::with_capacity(journal_entries.len()); @@ -6204,7 +6237,16 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { } } - (del_objects, del_errs) + // An accounting identity is actionable only when the delete result is + // successful. Never let a failed commit (including a partial quorum + // failure) reach the request-layer fast delta path. + for (index, err) in del_errs.iter().enumerate() { + if err.is_some() { + accounting[index] = None; + } + } + + (del_objects, del_errs, accounting) } #[tracing::instrument(skip(self))] @@ -6533,6 +6575,12 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { let mut obj_info = ObjectInfo::from_file_info(&dfi, bucket, object, opts.versioned || opts.version_suspended); obj_info.size = goi.size; + // Keep the committed source metadata on the internal delete result so + // the request layer can derive canonical accounting for this exact + // generation. Delete responses do not expose these fields. + obj_info.actual_size = goi.actual_size; + obj_info.user_defined = Arc::clone(&goi.user_defined); + obj_info.parts = Arc::clone(&goi.parts); obj_info.user_tags = Arc::clone(&goi.user_tags); self.invalidate_get_object_metadata_cache(bucket, object).await; Ok(obj_info) @@ -7824,6 +7872,113 @@ mod replication_quota_safety_tests { assert_eq!(stored.get_actual_size().expect("stored logical size should parse"), 1); } + #[tokio::test] + async fn delete_returns_canonical_compressed_accounting_size() { + let (_temp_dirs, disks, set_disks) = hermetic_set_disks(4).await; + let bucket = "compressed-delete-accounting"; + for disk in &disks { + disk.make_volume(bucket).await.expect("bucket volume should be created"); + } + + let mut user_defined = HashMap::new(); + insert_str( + &mut user_defined, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + insert_str(&mut user_defined, SUFFIX_ACTUAL_SIZE, "1000".to_string()); + let mut reader = PutObjReader::new( + HashReader::from_stream(Cursor::new(vec![0x5a; 400]), 400, 1000, None, None, false) + .expect("compressed fixture reader should be valid"), + ); + set_disks + .put_object( + bucket, + "object", + &mut reader, + &ObjectOptions { + user_defined, + ..Default::default() + }, + ) + .await + .expect("compressed object should be written"); + + let (deleted, errors, accounting) = set_disks + .delete_objects_with_accounting( + bucket, + vec![ObjectToDelete { + object_name: "object".to_string(), + ..Default::default() + }], + ObjectOptions { + object_lock_config_snapshot: Some(Arc::new(ObjectLockConfigSnapshot::new( + ObjectLockConfigState::ConfirmedAbsent, + ))), + ..Default::default() + }, + ) + .await; + + assert!(errors[0].is_none(), "compressed delete should succeed: {:?}", errors[0]); + assert!(deleted[0].found, "the committed object must be reported as found"); + assert_eq!(accounting[0].as_ref().and_then(|value| value.size), Some(1000)); + assert!(accounting[0].as_ref().is_some_and(|value| value.version_id.is_none())); + assert!(accounting[0].as_ref().is_some_and(|value| value.removed_current_object)); + } + + #[tokio::test] + async fn suspended_delete_marker_does_not_return_body_accounting() { + let (_temp_dirs, disks, set_disks) = hermetic_set_disks(4).await; + let bucket = "suspended-delete-accounting"; + for disk in &disks { + disk.make_volume(bucket).await.expect("bucket volume should be created"); + } + + let mut user_defined = HashMap::new(); + insert_str( + &mut user_defined, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + insert_str(&mut user_defined, SUFFIX_ACTUAL_SIZE, "1000".to_string()); + let mut reader = PutObjReader::new( + HashReader::from_stream(Cursor::new(vec![0x5a; 400]), 400, 1000, None, None, false) + .expect("compressed fixture reader should be valid"), + ); + let suspended_opts = ObjectOptions { + version_suspended: true, + delete_replication_config_snapshot: Some(Arc::new(DeleteReplicationConfigSnapshot::from_configs_for_test( + s3s::dto::VersioningConfiguration { + status: Some(s3s::dto::BucketVersioningStatus::from_static(s3s::dto::BucketVersioningStatus::SUSPENDED)), + ..Default::default() + }, + None, + ))), + user_defined, + object_lock_config_snapshot: Some(Arc::new(ObjectLockConfigSnapshot::new(ObjectLockConfigState::ConfirmedAbsent))), + ..Default::default() + }; + set_disks + .put_object(bucket, "object", &mut reader, &suspended_opts) + .await + .expect("compressed object should be written"); + + let (deleted, errors, accounting) = set_disks + .delete_objects_with_accounting( + bucket, + vec![ObjectToDelete { + object_name: "object".to_string(), + ..Default::default() + }], + suspended_opts, + ) + .await; + assert!(errors[0].is_none(), "suspended delete should create a marker: {:?}", errors[0]); + assert!(deleted[0].delete_marker); + assert!(accounting[0].is_none(), "a delete marker must not carry body accounting"); + } + #[tokio::test] async fn direct_put_cannot_persist_a_tiny_logical_size() { let (_temp_dirs, disks, set_disks) = hermetic_set_disks(4).await; diff --git a/crates/ecstore/src/storage_api_contracts/mod.rs b/crates/ecstore/src/storage_api_contracts/mod.rs index 11e7b800f..78a3f9c2d 100644 --- a/crates/ecstore/src/storage_api_contracts/mod.rs +++ b/crates/ecstore/src/storage_api_contracts/mod.rs @@ -62,8 +62,8 @@ pub(crate) mod object { use super::{Debug, Error, FileInfo, GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader}; use crate::storage_api_contracts::range::HTTPRangeSpec; pub(crate) use rustfs_storage_api::{ - DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockDeleteOptions, ObjectLockRetentionOptions, ObjectOperations, - ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState, ObjectToDelete, + DeleteAccounting, DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockDeleteOptions, ObjectLockRetentionOptions, + ObjectOperations, ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState, ObjectToDelete, }; pub(crate) trait EcstoreObjectIO: diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 312a25662..e5f2f0465 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -41,7 +41,7 @@ use crate::set_disk::{ }; use crate::storage_api_contracts::{ namespace::NamespaceLocking as _, - object::{ObjectIO as _, ObjectOperations as _}, + object::{DeleteAccounting, ObjectIO as _, ObjectOperations as _}, }; use parking_lot::Mutex as ParkingMutex; use rustfs_io_metrics::{ @@ -1216,6 +1216,14 @@ fn return_batch_delete_lock_error(objects: &[ObjectToDelete], err: Error) -> (Ve (del_objects, del_errs) } +fn return_batch_delete_lock_error_with_accounting( + objects: &[ObjectToDelete], + err: Error, +) -> (Vec, Vec>, Vec>) { + let (deleted, errors) = return_batch_delete_lock_error(objects, err); + (deleted, errors, vec![None; objects.len()]) +} + fn sorted_unique_delete_object_names(objects: &[ObjectToDelete]) -> Vec<&str> { let mut object_names: Vec<&str> = objects.iter().map(|object| object.object_name.as_str()).collect(); object_names.sort_unstable(); @@ -2312,6 +2320,22 @@ impl ECStore { result } + pub async fn delete_objects_with_tier_delete_journal_and_accounting( + self: &Arc, + bucket: &str, + objects: Vec, + opts: ObjectOptions, + ) -> (Vec, Vec>, Vec>) { + let result = self + .handle_delete_objects_with_journal_and_accounting(bucket, objects, opts, Some(Arc::clone(self))) + .await; + let success_count = result.1.iter().filter(|err| err.is_none()).count(); + if success_count > 0 { + list_objects::observe_list_objects_mutations(self, bucket, success_count).await; + } + result + } + #[instrument(skip(self))] pub(super) async fn handle_delete_object(&self, bucket: &str, object: &str, opts: ObjectOptions) -> Result { self.handle_delete_object_with_journal(bucket, object, opts, None).await @@ -2689,6 +2713,19 @@ impl ECStore { opts: ObjectOptions, tier_journal_api: Option>, ) -> (Vec, Vec>) { + let (deleted, errors, _) = self + .handle_delete_objects_with_journal_and_accounting(bucket, objects, opts, tier_journal_api) + .await; + (deleted, errors) + } + + pub(super) async fn handle_delete_objects_with_journal_and_accounting( + &self, + bucket: &str, + objects: Vec, + opts: ObjectOptions, + tier_journal_api: Option>, + ) -> (Vec, Vec>, Vec>) { // encode object name let objects: Vec = objects .iter() @@ -2701,6 +2738,7 @@ impl ECStore { // Default return value let mut del_objects = vec![DeletedObject::default(); objects.len()]; + let mut accounting = vec![None; objects.len()]; let mut del_errs = Vec::with_capacity(objects.len()); for _ in 0..objects.len() { @@ -2714,7 +2752,7 @@ impl ECStore { } else { match self.acquire_bucket_lifecycle_read_lock(bucket).await { Ok(guard) => Some(guard), - Err(err) => return return_batch_delete_lock_error(objects.as_slice(), err), + Err(err) => return return_batch_delete_lock_error_with_accounting(objects.as_slice(), err), } }; if let Some(guard) = _bucket_lifecycle_guard.as_ref() { @@ -2726,21 +2764,21 @@ impl ECStore { Err(err) => { let message = err.to_string(); let errors = (0..objects.len()).map(|_| Some(Error::other(message.clone()))).collect(); - return (del_objects, errors); + return (del_objects, errors, accounting); } } } if !is_meta_bucketname(bucket) && let Err(err) = get_cached_bucket_incarnation_id_in(&self.ctx, bucket).await { - return return_batch_delete_lock_error(objects.as_slice(), err); + return return_batch_delete_lock_error_with_accounting(objects.as_slice(), err); } let _object_lock_metadata_guard = if is_meta_bucketname(bucket) { None } else { Some(match acquire_bucket_metadata_transaction_read_lock_in(&self.ctx, bucket).await { Ok(guard) => guard, - Err(err) => return return_batch_delete_lock_error(objects.as_slice(), err), + Err(err) => return return_batch_delete_lock_error_with_accounting(objects.as_slice(), err), }) }; if let Some(guard) = _object_lock_metadata_guard.as_ref() { @@ -2750,7 +2788,7 @@ impl ECStore { let (state, incarnation_id, config_revision) = match get_object_lock_config_and_incarnation_from_disk_in(&self.ctx, bucket).await { Ok(snapshot) => snapshot, - Err(err) => return return_batch_delete_lock_error(objects.as_slice(), err), + Err(err) => return return_batch_delete_lock_error_with_accounting(objects.as_slice(), err), }; opts.object_lock_config_snapshot = Some(Arc::new(ObjectLockConfigSnapshot::for_store_bucket( self.id, @@ -2766,7 +2804,10 @@ impl ECStore { if let (Some(expected), Some(current)) = (opts.expected_bucket_incarnation_id, current_bucket_incarnation_id) && expected != current { - return return_batch_delete_lock_error(objects.as_slice(), StorageError::BucketNotFound(bucket.to_string())); + return return_batch_delete_lock_error_with_accounting( + objects.as_slice(), + StorageError::BucketNotFound(bucket.to_string()), + ); } #[cfg(test)] if current_bucket_incarnation_id.is_some() { @@ -2774,7 +2815,7 @@ impl ECStore { } let _object_lock_guards = match self.acquire_delete_objects_write_locks(bucket, &objects, &mut opts).await { Ok(guards) => guards, - Err(err) => return return_batch_delete_lock_error(objects.as_slice(), err), + Err(err) => return return_batch_delete_lock_error_with_accounting(objects.as_slice(), err), }; let mut futures = Vec::with_capacity(self.pools.len()); @@ -2783,22 +2824,24 @@ impl ECStore { if self.is_pool_rebalancing(pool.pool_idx).await { continue; } - futures.push(pool.delete_objects(bucket, objects.clone(), opts.clone())); + futures.push(pool.delete_objects_with_accounting(bucket, objects.clone(), opts.clone())); } let results = join_all(futures).await; for idx in 0..del_objects.len() { - for (dels, errs) in results.iter() { + for (dels, errs, pool_accounting) in results.iter() { if errs[idx].is_none() && dels[idx].found { del_errs[idx] = None; del_objects[idx] = dels[idx].clone(); + accounting[idx] = pool_accounting[idx].clone(); break; } if del_errs[idx].is_none() { del_errs[idx] = errs[idx].clone(); del_objects[idx] = dels[idx].clone(); + accounting[idx] = pool_accounting[idx].clone(); } } } @@ -2807,7 +2850,7 @@ impl ECStore { v.object_name = decode_dir_object(&v.object_name); }); - (del_objects, del_errs) + (del_objects, del_errs, accounting) // let mut futures = Vec::with_capacity(objects.len()); diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index e011327f9..1114349e1 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -76,6 +76,7 @@ pub use bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOption pub use capability::{CapabilitySnapshotError, CapabilityState, CapabilityStatus}; pub use error::{StorageErrorCode, StorageResult}; pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo}; +pub use object::DeleteAccounting; pub use object::ObjectLockDeleteOptions; pub use object::{DeletedObject, ObjectToDelete}; pub use object::{ExpirationOptions, TransitionedObject}; diff --git a/crates/storage-api/src/object.rs b/crates/storage-api/src/object.rs index 9f0957bde..7959354dc 100644 --- a/crates/storage-api/src/object.rs +++ b/crates/storage-api/src/object.rs @@ -218,6 +218,17 @@ pub struct DeletedObject { pub force_delete_generation: Option, } +/// Accounting identity returned by the internal commit-time delete path. +/// +/// This is carried separately from [`DeletedObject`] so adding quota details +/// does not change the source shape of the public S3 delete result contract. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct DeleteAccounting { + pub size: Option, + pub version_id: Option, + pub removed_current_object: bool, +} + impl DeletedObject { pub fn version_purge_status(&self) -> VersionPurgeStatusType { self.replication_state @@ -341,6 +352,19 @@ pub trait ObjectOperations: Send + Sync + fmt::Debug { objects: Vec, opts: Self::ObjectOptions, ) -> (Vec, Vec>); + /// Delete objects and optionally return commit-time accounting identities. + /// The default preserves the ordinary delete contract for implementations + /// that do not expose storage-level accounting details. + async fn delete_objects_with_accounting( + &self, + bucket: &str, + objects: Vec, + opts: Self::ObjectOptions, + ) -> (Vec, Vec>, Vec>) { + let object_count = objects.len(); + let (deleted, errors) = self.delete_objects(bucket, objects, opts).await; + (deleted, errors, vec![None; object_count]) + } async fn put_object_metadata( &self, bucket: &str, diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index 7d55035e9..b51254b23 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -1021,7 +1021,7 @@ fn build_list_objects_v2_metadata_output( object: Object { key: Some(encode_list_objects_v2_value(&object.name, encoding_type)), last_modified: object.mod_time.map(Timestamp::from), - size: Some(object.get_actual_size().unwrap_or_default()), + size: Some(object.get_actual_size_or_physical()), e_tag: object.etag.clone().map(|etag| to_s3s_etag(&etag)), storage_class: Some(ObjectStorageClass::from( object diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index a31ec1d19..d1978f2db 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -3969,6 +3969,55 @@ fn delete_creates_delete_marker(opts: &ObjectOptions) -> bool { opts.version_id.is_none() && opts.versioned && !opts.version_suspended } +fn delete_removes_current_object(opts: &ObjectOptions) -> bool { + delete_request_targets_current( + opts.version_id + .as_deref() + .and_then(|version_id| Uuid::parse_str(version_id).ok()), + ) +} + +fn delete_request_targets_current(version_id: Option) -> bool { + version_id.is_none() || version_id.is_some_and(|version_id| version_id.is_nil()) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DeleteMemoryUpdate { + DeleteMarker, + Object { size: u64, removed_current_object: bool }, +} + +fn delete_memory_update( + creates_delete_marker: bool, + committed_delete_marker: bool, + requested_current: bool, + accounting_size: Option, + removed_current_object: bool, +) -> Option { + if creates_delete_marker || (committed_delete_marker && requested_current) { + return Some(DeleteMemoryUpdate::DeleteMarker); + } + + (!committed_delete_marker) + .then_some(accounting_size) + .flatten() + .map(|size| DeleteMemoryUpdate::Object { + size, + removed_current_object, + }) +} + +async fn apply_delete_memory_update(bucket: &str, update: Option) { + match update { + Some(DeleteMemoryUpdate::DeleteMarker) => record_bucket_delete_marker_memory(bucket).await, + Some(DeleteMemoryUpdate::Object { + size, + removed_current_object, + }) => record_bucket_object_delete_memory(bucket, size, removed_current_object).await, + None => {} + } +} + /// `DeleteObjects` is idempotent. A raw filesystem `NotFound` can cross the /// distributed delete path instead of its usual typed missing-object error. fn is_delete_objects_not_found(error: &EcstoreError) -> bool { @@ -8409,8 +8458,6 @@ impl DefaultObjectUsecase { object: ObjectToDelete, versioned: bool, version_suspended: bool, - size: i64, - existing: Option, } // Phase 2 (bounded concurrency, backlog#929 / HP-8): collect the @@ -8428,32 +8475,23 @@ impl DefaultObjectUsecase { skip_stat, } = prepared; let synthetic_version_id = object.version_id.is_none() && is_dir_object(&object.object_name); - let (goi, source_missing) = if skip_stat { - (ObjectInfo::default(), false) - } else { + if !skip_stat { match store_ref.get_object_info(bucket_ref, &object.object_name, &opts).await { - Ok(res) => (res, false), - Err(err) if is_err_object_not_found(&err) || is_err_version_not_found(&err) => { - (ObjectInfo::default(), true) - } + Ok(_) => {} + Err(err) if is_err_object_not_found(&err) || is_err_version_not_found(&err) => {} Err(err) => return Err(ApiError::from(err)), } - }; - - let size = goi.size; + } if synthetic_version_id { object.version_id = Some(Uuid::nil()); } - let existing = (!skip_stat && !source_missing).then_some(goi); Ok::<_, ApiError>(AdmittedDelete { idx, object, versioned: opts.versioned, version_suspended: opts.version_suspended, - size, - existing, }) })) .buffered(DELETE_OBJECTS_PRE_STAT_CONCURRENCY) @@ -8464,15 +8502,11 @@ impl DefaultObjectUsecase { // per-key success/failure reporting is unchanged. let mut object_to_delete = Vec::new(); let mut object_to_delete_idx = Vec::new(); - let mut object_sizes = Vec::new(); - let mut existing_object_infos = Vec::new(); let mut object_versioning = Vec::new(); for admitted in admitted_deletes { - object_sizes.push(admitted.size); object_to_delete_idx.push(admitted.idx); object_versioning.push((admitted.versioned, admitted.version_suspended)); object_to_delete.push(admitted.object); - existing_object_infos.push(admitted.existing); } let cache_adapter = self.object_data_cache(); let cache_keys_before_delete = object_to_delete @@ -8489,8 +8523,8 @@ impl DefaultObjectUsecase { ..Default::default() }; apply_bucket_generation_guard(&req, &bucket, &mut storage_delete_opts)?; - let (dobjs, errs) = store - .delete_objects_with_tier_delete_journal(&bucket, object_to_delete.clone(), storage_delete_opts) + let (dobjs, errs, accounting) = store + .delete_objects_with_tier_delete_journal_and_accounting(&bucket, object_to_delete.clone(), storage_delete_opts) .await; let _manager = get_concurrency_manager(); @@ -8515,17 +8549,16 @@ impl DefaultObjectUsecase { delete_results[didx].delete_object = Some(deleted_object.clone()); let (versioned, version_suspended) = object_versioning[i]; let creates_delete_marker = object_to_delete[i].version_id.is_none() && versioned && !version_suspended; - if creates_delete_marker { - record_bucket_delete_marker_memory(&bucket).await; - } else { - let size = object_sizes[i].max(0) as u64; - record_bucket_object_delete_memory( - &bucket, - size, - existing_object_infos[i].is_some() && object_to_delete[i].version_id.is_none(), - ) - .await; - } + let committed_delete_marker = dobjs[i].delete_marker; + let delete_accounting = accounting.get(i).and_then(Option::as_ref); + let update = delete_memory_update( + creates_delete_marker, + committed_delete_marker, + delete_request_targets_current(object_to_delete[i].version_id), + delete_accounting.and_then(|value| value.size), + delete_accounting.is_some_and(|value| value.removed_current_object), + ); + apply_delete_memory_update(&bucket, update).await; } Err(error) => { delete_results[didx].error = Some(error); @@ -8803,12 +8836,24 @@ impl DefaultObjectUsecase { let _ = invalidate_object_data_cache_after_delete_success(&cache_adapter, &bucket, &key).await; } - // Fast in-memory update for immediate quota and admin usage consistency - if delete_creates_delete_marker(&opts) { - record_bucket_delete_marker_memory(&bucket).await; + // Fast in-memory update for immediate quota and admin usage consistency. + // Prefix/force deletes and synthetic directory entries do not carry one + // committed object identity; leave their cache delta to reconciliation. + let update = if force_delete || obj_info.name.is_empty() || synthetic_version_id { + None } else { - record_bucket_object_delete_memory(&bucket, obj_info.size.max(0) as u64, opts.version_id.is_none()).await; - } + // The storage commit returns this object's metadata while its + // generation lock is held. Never fall back to a pre-delete stat: + // an overwrite can commit between that stat and this delete. + delete_memory_update( + delete_creates_delete_marker(&opts), + obj_info.delete_marker, + opts.version_id.is_none(), + quota_object_size(&obj_info).ok(), + delete_removes_current_object(&opts), + ) + }; + apply_delete_memory_update(&bucket, update).await; if obj_info.name.is_empty() { if let Some((operation_id, target_arns, generation)) = force_delete_intent { @@ -17861,6 +17906,158 @@ mod tests { assert!(!can_skip_delete_objects_pre_stat(false, &delete_marker_creating_opts(), false)); } + #[test] + fn delete_accounting_recognizes_explicit_null_as_current_object() { + let opts = ObjectOptions { + version_id: Some(Uuid::nil().to_string()), + version_suspended: true, + ..Default::default() + }; + assert!(delete_removes_current_object(&opts)); + assert!(delete_request_targets_current(Some(Uuid::nil()))); + assert!(!delete_request_targets_current(Some(Uuid::new_v4()))); + assert!(!delete_removes_current_object(&ObjectOptions { + version_id: Some(Uuid::new_v4().to_string()), + ..Default::default() + })); + } + + #[test] + fn compressed_object_delete_restores_usage_baseline() { + let mut metadata = HashMap::new(); + insert_str(&mut metadata, SUFFIX_COMPRESSION, "klauspost/compress/s2".to_string()); + let object = ObjectInfo { + size: 400, + actual_size: 1000, + user_defined: Arc::new(metadata), + ..Default::default() + }; + let accounting_size = quota_object_size(&object).expect("logical compressed size should be canonical"); + + assert_eq!( + delete_memory_update(false, false, true, Some(accounting_size), true), + Some(DeleteMemoryUpdate::Object { + size: 1000, + removed_current_object: true, + }) + ); + } + + #[test] + fn invalid_accounting_metadata_is_reconciled_without_overflow() { + assert_eq!(delete_memory_update(false, false, true, None, true), None); + assert_eq!( + delete_memory_update(false, true, true, None, true), + Some(DeleteMemoryUpdate::DeleteMarker) + ); + } + + #[tokio::test] + #[serial_test::serial] + async fn compressed_delete_requests_restore_usage_baseline() { + use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, DeleteBucketOptions, MakeBucketOptions}; + + let store = crate::app::gating_test_env::shared_gating_ecstore().await; + if current_app_context().is_none() { + crate::app::runtime_sources::install_test_app_context(Arc::clone(&store)).await; + } + let bucket = format!("compressed-delete-request-{}", Uuid::new_v4().simple()); + store + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("create compressed delete request bucket"); + + // Seed the process-local usage with the canonical logical bytes. The + // direct storage PUT below intentionally does not apply an app-layer + // usage delta; the two real DELETE requests must remove exactly this + // amount through their request-layer wiring. + crate::app::storage_api::test::data_usage::seed_bucket_usage_memory_for_test(&bucket, 2_000).await; + + for object in ["single", "batch"] { + let mut metadata = HashMap::new(); + insert_str(&mut metadata, SUFFIX_COMPRESSION, "klauspost/compress/s2".to_string()); + insert_str(&mut metadata, SUFFIX_ACTUAL_SIZE, "1000".to_string()); + let reader = HashReader::from_stream(std::io::Cursor::new(vec![0x5a; 400]), 400, 1000, None, None, false) + .expect("compressed fixture reader should be valid"); + let mut reader = PutObjReader::new(reader); + store + .put_object( + &bucket, + object, + &mut reader, + &ObjectOptions { + user_defined: metadata, + ..Default::default() + }, + ) + .await + .expect("compressed fixture object should be written"); + } + + let mut single_req = build_request( + DeleteObjectInput::builder() + .bucket(bucket.clone()) + .key("single".to_string()) + .build() + .expect("single delete input should build"), + Method::DELETE, + ); + single_req.extensions.insert(crate::storage::access::ReqInfo { + cred: Some(rustfs_credentials::Credentials::default()), + is_owner: true, + ..Default::default() + }); + DefaultObjectUsecase::from_global() + .execute_delete_object(single_req) + .await + .expect("single compressed delete should succeed"); + assert_eq!( + crate::app::storage_api::test::data_usage::get_bucket_usage_memory(&bucket).await, + Some(1_000), + "single delete must subtract the logical accounting size" + ); + + let mut batch_req = build_request( + DeleteObjectsInput::builder() + .bucket(bucket.clone()) + .delete(Delete { + objects: vec![ObjectIdentifier { + key: "batch".to_string(), + ..Default::default() + }], + quiet: None, + }) + .build() + .expect("batch delete input should build"), + Method::POST, + ); + batch_req.extensions.insert(crate::storage::access::ReqInfo { + cred: Some(rustfs_credentials::Credentials::default()), + is_owner: true, + ..Default::default() + }); + DefaultObjectUsecase::from_global() + .execute_delete_objects(batch_req) + .await + .expect("batch compressed delete should succeed"); + assert_eq!( + crate::app::storage_api::test::data_usage::get_bucket_usage_memory(&bucket).await, + Some(0), + "batch delete must subtract the committed logical accounting size" + ); + + store + .delete_bucket( + &bucket, + &DeleteBucketOptions { + force: true, + ..Default::default() + }, + ) + .await + .expect("clean up compressed delete request bucket"); + } + #[tokio::test] async fn execute_get_object_attributes_returns_internal_error_when_store_uninitialized() { let input = GetObjectAttributesInput::builder() diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index d90cfb6a8..a599cfde6 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -72,6 +72,11 @@ pub(crate) mod data_usage { compute_bucket_usage, live_bucket_usage_computations, seed_bucket_usage_memory_for_test, store_data_usage_in_backend, }; + #[cfg(test)] + pub(crate) async fn get_bucket_usage_memory(bucket: &str) -> Option { + crate::storage::storage_api::ecstore_data_usage::get_bucket_usage_memory(bucket).await + } + pub(crate) async fn record_bucket_object_delete_memory(bucket: &str, deleted_size: u64, removed_current_object: bool) { crate::storage::storage_api::ecstore_data_usage::record_bucket_object_delete_memory( bucket, @@ -1233,7 +1238,10 @@ pub(crate) mod test { pub(crate) use super::access::ReqInfo; pub(crate) use super::options::VERSIONING_CONFIG_LOOKUPS; - pub(crate) use super::{bucket, data_usage, ecfs, object_utils, runtime}; + pub(crate) use super::{bucket, ecfs, object_utils, runtime}; + pub(crate) mod data_usage { + pub(crate) use super::super::data_usage::*; + } pub(crate) use crate::storage::storage_api::test_consumer::{get_global_bucket_metadata_sys, set_bucket_metadata}; pub(crate) use crate::storage::storage_api::{ ECStore, Endpoint, Endpoints, PoolEndpoints, StorageObjectInfo, StorageObjectOptions, StoragePutObjReader, diff --git a/rustfs/src/storage/s3_api/bucket.rs b/rustfs/src/storage/s3_api/bucket.rs index e105adba4..6599ca107 100644 --- a/rustfs/src/storage/s3_api/bucket.rs +++ b/rustfs/src/storage/s3_api/bucket.rs @@ -296,7 +296,10 @@ pub(crate) fn build_list_objects_v2_output( let mut obj = Object { key: Some(key), last_modified: v.mod_time.map(Timestamp::from), - size: Some(v.get_actual_size().unwrap_or_default()), + // Compressed legacy objects may retain an unknown (-1) + // logical-size sentinel; never expose that internal value in + // an S3 response. + size: Some(v.get_actual_size_or_physical()), e_tag: v.etag.clone().map(|etag| to_s3s_etag(&etag)), storage_class: v.storage_class.clone().map(ObjectStorageClass::from), ..Default::default() @@ -656,6 +659,45 @@ mod tests { assert_eq!(output.common_prefixes.as_ref().map(std::vec::Vec::len), Some(2)); } + #[test] + fn list_objects_never_exposes_compressed_unknown_size_sentinel() { + let mut metadata = std::collections::HashMap::new(); + rustfs_utils::http::insert_str( + &mut metadata, + rustfs_utils::http::SUFFIX_COMPRESSION, + "klauspost/compress/s2".to_string(), + ); + let output = build_list_objects_v2_output( + ListObjectsV2Info { + objects: vec![ObjectInfo { + name: "legacy-compressed".to_string(), + size: 128, + actual_size: -1, + user_defined: std::sync::Arc::new(metadata), + ..Default::default() + }], + ..Default::default() + }, + false, + 1000, + "bucket".to_string(), + String::new(), + None, + None, + None, + None, + ); + + assert_eq!( + output + .contents + .as_ref() + .and_then(|objects| objects.first()) + .and_then(|object| object.size), + Some(128) + ); + } + #[test] fn list_responses_report_standard_for_legacy_label_only_file_metadata() { let version_id = Uuid::parse_str("11111111-2222-3333-4444-555555555555").expect("fixture version ID should be valid"); diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 31668783a..e5277c0dd 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -429,6 +429,8 @@ pub(crate) mod ecstore_config { } pub(crate) mod ecstore_data_usage { + #[cfg(test)] + pub(crate) use rustfs_ecstore::api::data_usage::get_bucket_usage_memory; pub(crate) use rustfs_ecstore::api::data_usage::{ apply_bucket_usage_memory_overlay, init_compression_total_memory_from_backend, load_admin_data_usage_from_backend_cached, load_data_usage_from_backend, quota_object_size, record_bucket_delete_marker_memory, record_bucket_object_delete_memory, From a930152d5a5cc38320effd0231a82288278ff08b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Sat, 22 Aug 2026 20:43:52 +0800 Subject: [PATCH 3/8] fix(admin): expose per-target disableProxy through remote target admin API (#6376) The read-proxy selector already honors a target's disable_proxy flag (PR #6172), but the admin API still rejected the field, so the only way to set it was importing a MinIO-written bucket-targets.json. - move disableProxy from REMOTE_TARGET_UNSUPPORTED_FIELDS to REMOTE_TARGET_WRITABLE_FIELDS (set-remote-target create accepts it) - add TargetUpdateOp::Proxy so set-remote-target?update=true&proxy=true overlays only the proxy group (MinIO TargetUpdateType parity) - bump REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION 1 -> 2 and update the runtime capability pin tests - keep edge/edgeSyncBeforeExpiry rejected (no implementation behind them) - pin that a published TargetClient carries disable_proxy, the field the proxy-target selector consults Refs rustfs/backlog#1950 --- .../ecstore/src/bucket/bucket_target_sys.rs | 38 +++++++++++++ crates/replication/src/config.rs | 9 ++- rustfs/src/admin/handlers/replication.rs | 56 ++++++++++++++++--- rustfs/src/admin/handlers/system.rs | 25 +++++++-- 4 files changed, 115 insertions(+), 13 deletions(-) diff --git a/crates/ecstore/src/bucket/bucket_target_sys.rs b/crates/ecstore/src/bucket/bucket_target_sys.rs index 74bf96d7f..7e5a5df5f 100644 --- a/crates/ecstore/src/bucket/bucket_target_sys.rs +++ b/crates/ecstore/src/bucket/bucket_target_sys.rs @@ -3425,6 +3425,44 @@ mod tests { assert!(mutexes.contains_key("second")); } + #[tokio::test] + async fn update_all_targets_publishes_disable_proxy_on_target_client() { + // The read-proxy selector (replication_proxy::get_proxy_targets) skips + // targets whose TargetClient carries disable_proxy — the persisted + // per-target opt-out must survive client publication. + let sys = BucketTargetSys::default(); + let target = |arn: &str, disable_proxy: bool| BucketTarget { + arn: arn.to_string(), + endpoint: "192.168.1.10:9000".to_string(), + target_bucket: "target-bucket".to_string(), + region: "us-east-1".to_string(), + disable_proxy, + credentials: Some(Credentials { + access_key: "access".to_string(), + secret_key: "secret".to_string(), + session_token: None, + expiration: None, + }), + ..Default::default() + }; + let targets = BucketTargets { + targets: vec![target("arn:proxied", false), target("arn:opted-out", true)], + }; + + sys.update_all_targets("bucket", Some(&targets)).await; + + let proxied = sys + .get_remote_target_client("bucket", "arn:proxied") + .await + .expect("client should be published"); + assert!(!proxied.disable_proxy); + let opted_out = sys + .get_remote_target_client("bucket", "arn:opted-out") + .await + .expect("client should be published"); + assert!(opted_out.disable_proxy, "disable_proxy must reach the published TargetClient"); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn target_updates_serialize_client_build_through_publication_per_bucket() { let sys = Arc::new(BucketTargetSys::default()); diff --git a/crates/replication/src/config.rs b/crates/replication/src/config.rs index c8ca328a5..405799f94 100644 --- a/crates/replication/src/config.rs +++ b/crates/replication/src/config.rs @@ -60,7 +60,9 @@ pub const REPLICATION_READ_ONLY_HISTORICAL_FIELDS: &[&str] = &[ "Destination.ReplicationTime", ]; -pub const REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION: u32 = 1; +// v2: disableProxy moved from unsupported to writable (per-target read-proxy +// opt-out is accepted by set-remote-target and the `proxy` update op). +pub const REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION: u32 = 2; pub const REMOTE_TARGET_WRITABLE_FIELDS: &[&str] = &[ "sourcebucket", @@ -83,9 +85,12 @@ pub const REMOTE_TARGET_WRITABLE_FIELDS: &[&str] = &[ // madmin default of 60s); the per-target health-check interval is not // yet applied — the heartbeat keeps its global env-configured interval. "healthCheckDuration", + // Per-target read-proxy opt-out, consumed by the proxy-target selector + // (contract v2; previously only importable via MinIO bucket-targets.json). + "disableProxy", ]; -pub const REMOTE_TARGET_UNSUPPORTED_FIELDS: &[&str] = &["disableProxy", "edge", "edgeSyncBeforeExpiry"]; +pub const REMOTE_TARGET_UNSUPPORTED_FIELDS: &[&str] = &["edge", "edgeSyncBeforeExpiry"]; #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct ObjectOpts { diff --git a/rustfs/src/admin/handlers/replication.rs b/rustfs/src/admin/handlers/replication.rs index 106c25790..229c3c9fd 100644 --- a/rustfs/src/admin/handlers/replication.rs +++ b/rustfs/src/admin/handlers/replication.rs @@ -73,6 +73,8 @@ enum TargetUpdateOp { /// Connection group: credentials plus endpoint, target bucket, and TLS settings. Credentials, Sync, + /// Per-target read-proxy opt-out (`disableProxy`). + Proxy, Bandwidth, Path, } @@ -81,12 +83,13 @@ fn parse_remote_target_update_ops(queries: &HashMap) -> S3Result const SUPPORTED_OPS: &[(&str, TargetUpdateOp)] = &[ ("creds", TargetUpdateOp::Credentials), ("sync", TargetUpdateOp::Sync), + ("proxy", TargetUpdateOp::Proxy), ("bandwidth", TargetUpdateOp::Bandwidth), ("path", TargetUpdateOp::Path), ]; // Present in the MinIO wire contract, but they drive target fields this // version rejects as unsupported — fail loudly instead of silently ignoring. - const UNSUPPORTED_OPS: &[&str] = &["proxy", "healthcheck", "edge", "edgeSyncBeforeExpiry"]; + const UNSUPPORTED_OPS: &[&str] = &["healthcheck", "edge", "edgeSyncBeforeExpiry"]; for key in UNSUPPORTED_OPS { if queries.get(*key).is_some_and(|value| value == "true") { @@ -312,11 +315,10 @@ impl RemoteTargetRequest { )); } - for (unsupported, configured) in - REMOTE_TARGET_UNSUPPORTED_FIELDS - .iter() - .copied() - .zip([self.disable_proxy, self.edge, self.edge_sync_before_expiry]) + for (unsupported, configured) in REMOTE_TARGET_UNSUPPORTED_FIELDS + .iter() + .copied() + .zip([self.edge, self.edge_sync_before_expiry]) { if configured { return Err(s3_error!( @@ -702,6 +704,7 @@ impl Operation for SetRemoteTargetHandler { target.deployment_id = remote_target.deployment_id.clone(); } TargetUpdateOp::Sync => target.replication_sync = remote_target.replication_sync, + TargetUpdateOp::Proxy => target.disable_proxy = remote_target.disable_proxy, TargetUpdateOp::Bandwidth => target.bandwidth_limit = remote_target.bandwidth_limit, TargetUpdateOp::Path => target.path = remote_target.path.clone(), } @@ -1520,6 +1523,7 @@ mod tests { ("update", "true"), ("creds", "true"), ("sync", "true"), + ("proxy", "true"), ("bandwidth", "true"), ("path", "true"), ])) @@ -1529,6 +1533,7 @@ mod tests { vec![ TargetUpdateOp::Credentials, TargetUpdateOp::Sync, + TargetUpdateOp::Proxy, TargetUpdateOp::Bandwidth, TargetUpdateOp::Path ] @@ -2070,7 +2075,6 @@ mod tests { ("credentials.session_token", serde_json::json!("session-token")), ("credentials.expiration", serde_json::json!("2026-01-01T00:00:00Z")), ("api", serde_json::json!("s3v2")), - ("disableProxy", serde_json::json!(true)), ("edge", serde_json::json!(true)), ("edgeSyncBeforeExpiry", serde_json::json!(true)), ] { @@ -2300,6 +2304,44 @@ mod tests { assert!(!REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"healthCheckDuration")); } + #[test] + fn remote_target_disable_proxy_is_declared_writable_edge_stays_unsupported() { + assert!(REMOTE_TARGET_WRITABLE_FIELDS.contains(&"disableProxy")); + assert!(!REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"disableProxy")); + // edge sync has no implementation behind it — it must stay rejected. + assert!(REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"edge")); + assert!(REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"edgeSyncBeforeExpiry")); + } + + #[test] + fn remote_target_create_accepts_disable_proxy() { + let mut request = valid_remote_target_request(); + request["disableProxy"] = serde_json::json!(true); + + let target = serde_json::from_value::(request) + .expect("request should deserialize") + .into_bucket_target() + .expect("disableProxy is a supported per-target read-proxy opt-out"); + + assert!(target.disable_proxy); + } + + #[test] + fn update_body_with_proxy_op_toggles_disable_proxy_without_credentials() { + // Mirrors the other partial-update groups: a proxy-only update body may + // omit the connection fields entirely. + let body = serde_json::json!({ + "arn": "arn:rustfs:replication:us-east-1:dep:target", + "type": "replication", + "disableProxy": true + }); + let request: RemoteTargetRequest = serde_json::from_value(body).expect("partial update body should deserialize"); + let target = request + .into_update_bucket_target(&[TargetUpdateOp::Proxy]) + .expect("proxy-only update must not require credentials"); + assert!(target.disable_proxy); + } + #[test] fn remote_target_capability_fields_do_not_overlap() { for field in REMOTE_TARGET_UNSUPPORTED_FIELDS { diff --git a/rustfs/src/admin/handlers/system.rs b/rustfs/src/admin/handlers/system.rs index 9df481c11..981baaefb 100644 --- a/rustfs/src/admin/handlers/system.rs +++ b/rustfs/src/admin/handlers/system.rs @@ -1262,7 +1262,9 @@ mod tests { assert_eq!(response.summary.manual_transition_jobs.state, CapabilityState::Supported); assert_eq!(response.replication.contract_version, 1); assert_eq!(response.replication.bucket_replication.contract_version, 1); - assert_eq!(response.replication.remote_targets.contract_version, 1); + // v2: disableProxy moved from unsupported to writable (per-target + // read-proxy opt-out reached the admin API). + assert_eq!(response.replication.remote_targets.contract_version, 2); assert_eq!(response.replication.bucket_replication.status.state, CapabilityState::Supported); assert_eq!(response.replication.remote_targets.status.state, CapabilityState::Supported); assert_eq!( @@ -1293,7 +1295,15 @@ mod tests { .remote_targets .fields .iter() - .any(|field| field.name == "disableProxy" && field.state == super::ReplicationFieldState::Unsupported) + .any(|field| field.name == "disableProxy" && field.state == super::ReplicationFieldState::Supported) + ); + assert!( + response + .replication + .remote_targets + .fields + .iter() + .any(|field| field.name == "edge" && field.state == super::ReplicationFieldState::Unsupported) ); assert!( response @@ -1364,7 +1374,7 @@ mod tests { assert_eq!(value["summary"]["manual_transition_jobs"]["state"], "supported"); assert_eq!(value["replication"]["contract_version"], 1); assert_eq!(value["replication"]["bucket_replication"]["contract_version"], 1); - assert_eq!(value["replication"]["remote_targets"]["contract_version"], 1); + assert_eq!(value["replication"]["remote_targets"]["contract_version"], 2); assert_eq!(value["replication"]["bucket_replication"]["status"]["state"], "supported"); assert_eq!(value["replication"]["remote_targets"]["status"]["state"], "supported"); assert_eq!( @@ -1383,7 +1393,14 @@ mod tests { .as_array() .expect("remote target fields should be an array") .iter() - .any(|field| field["name"] == "disableProxy" && field["state"] == "unsupported") + .any(|field| field["name"] == "disableProxy" && field["state"] == "supported") + ); + assert!( + value["replication"]["remote_targets"]["fields"] + .as_array() + .expect("remote target fields should be an array") + .iter() + .any(|field| field["name"] == "edge" && field["state"] == "unsupported") ); assert!( value["replication"]["remote_targets"]["fields"] From da90d02c150306364020d146f6fe709e74a6e85e Mon Sep 17 00:00:00 2001 From: cxymds Date: Sat, 22 Aug 2026 20:45:03 +0800 Subject: [PATCH 4/8] test(ecstore): cover suspended-owner heal semantics (#6348) * test(ecstore): cover suspended-owner heal semantics * test(heal): cover suspended owner production path --- crates/ecstore/src/store/heal.rs | 278 ++++++++++++++++++++++++++++++- 1 file changed, 276 insertions(+), 2 deletions(-) diff --git a/crates/ecstore/src/store/heal.rs b/crates/ecstore/src/store/heal.rs index d10abe740..ffac77751 100644 --- a/crates/ecstore/src/store/heal.rs +++ b/crates/ecstore/src/store/heal.rs @@ -297,10 +297,16 @@ impl ECStore { #[cfg(test)] mod tests { use super::*; + use crate::bucket::metadata_sys; use crate::core::pools::{PoolDecommissionInfo, PoolStatus}; - use crate::disk::{DiskOption, format::FormatV3, new_disk}; - use crate::layout::endpoints::{Endpoints, PoolEndpoints}; + use crate::disk::{DeleteOptions, DiskOption, format::FormatV3, new_disk}; + use crate::layout::endpoints::{EndpointServerPools, Endpoints, PoolEndpoints}; + use crate::runtime::instance::InstanceContext; + use crate::storage_api_contracts::bucket::{BucketOperations, MakeBucketOptions}; + use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations}; use crate::store::init_format::{load_format_erasure, save_format_file}; + use crate::store::init_local_disks_with_instance_ctx; + use tokio_util::sync::CancellationToken; async fn minimal_heal_pool(pool_idx: usize) -> Arc { let format = FormatV3::new(1, 1); @@ -347,6 +353,51 @@ mod tests { } } + async fn multi_pool_heal_store() -> (tempfile::TempDir, Arc, CancellationToken) { + let temp_dir = tempfile::tempdir().expect("multi-pool heal test directory should be created"); + let mut pool_endpoints = Vec::new(); + for pool_index in 0..2 { + let mut endpoints = Vec::new(); + for disk_index in 0..4 { + let disk_path = temp_dir.path().join(format!("pool{pool_index}-disk{disk_index}")); + tokio::fs::create_dir_all(&disk_path) + .await + .expect("multi-pool heal test disk should be created"); + let mut endpoint = Endpoint::try_from(disk_path.to_str().expect("disk path should be utf8")) + .expect("test endpoint should parse"); + endpoint.set_pool_index(pool_index); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + endpoints.push(endpoint); + } + pool_endpoints.push(PoolEndpoints { + legacy: false, + set_count: 1, + drives_per_set: 4, + endpoints: Endpoints::from(endpoints), + cmd_line: format!("heal-owner-pool-{pool_index}"), + platform: "test".to_string(), + }); + } + + let endpoint_pools = EndpointServerPools::from(pool_endpoints); + let instance_ctx = Arc::new(InstanceContext::new()); + init_local_disks_with_instance_ctx(&instance_ctx, endpoint_pools.clone()) + .await + .expect("multi-pool local disks should initialize"); + let shutdown = CancellationToken::new(); + let store = ECStore::new_with_instance_ctx( + "127.0.0.1:0".parse().expect("test address should parse"), + endpoint_pools, + shutdown.clone(), + instance_ctx, + ) + .await + .expect("multi-pool test store should initialize"); + metadata_sys::init_bucket_metadata_sys(store.clone(), Vec::new()).await; + (temp_dir, store, shutdown) + } + #[tokio::test] async fn heal_object_pool_scope_selects_only_requested_pool() { let store = minimal_heal_store().await; @@ -506,6 +557,229 @@ mod tests { } } + #[tokio::test] + #[serial_test::serial] + async fn unscoped_heal_object_suspended_owner_semantics() { + let (_temp_dir, store, shutdown) = multi_pool_heal_store().await; + let bucket = format!("heal-owner-{}", Uuid::new_v4().simple()); + let active_object = "active-owner"; + let suspended_only_object = "suspended-only"; + let duplicate_object = "duplicate-owner"; + let marker_object = "marker-owner"; + let quorum_object = "quorum-owner"; + store + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created in all pools"); + + let mut active_reader = PutObjReader::from_vec(b"active owner".to_vec()); + store.pools[0] + .put_object(&bucket, active_object, &mut active_reader, &ObjectOptions::default()) + .await + .expect("active owner object should be written"); + let active_disks = store.pools[0].disk_set[0].disks.read().await.clone(); + let missing_active_disk = active_disks[0].clone().expect("active disk should be online"); + missing_active_disk + .delete( + &bucket, + active_object, + DeleteOptions { + recursive: true, + immediate: true, + ..Default::default() + }, + ) + .await + .expect("active owner shard should be removed for repair"); + assert!( + missing_active_disk.read_xl(&bucket, active_object, false).await.is_err(), + "the active owner fixture must start with one missing metadata copy" + ); + + let mut suspended_reader = PutObjReader::from_vec(b"suspended owner".to_vec()); + store.pools[1] + .put_object(&bucket, suspended_only_object, &mut suspended_reader, &ObjectOptions::default()) + .await + .expect("suspended owner object should be written"); + for (pool_index, mod_time) in [1_i64, 2_i64].into_iter().enumerate() { + let mut duplicate_reader = PutObjReader::from_vec(format!("duplicate-pool-{pool_index}").into_bytes()); + store.pools[pool_index] + .put_object( + &bucket, + duplicate_object, + &mut duplicate_reader, + &ObjectOptions { + mod_time: Some(OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(mod_time)), + ..Default::default() + }, + ) + .await + .expect("duplicate owner object should be written"); + } + let duplicate_missing_disk = store.pools[0].disk_set[0].disks.read().await[0] + .clone() + .expect("duplicate active owner disk should be online"); + duplicate_missing_disk + .delete( + &bucket, + duplicate_object, + DeleteOptions { + recursive: true, + immediate: true, + ..Default::default() + }, + ) + .await + .expect("duplicate active owner shard should be removed for repair"); + let history_version = Uuid::new_v4(); + let mut history_reader = PutObjReader::from_vec(b"marker history".to_vec()); + store.pools[0] + .put_object( + &bucket, + marker_object, + &mut history_reader, + &ObjectOptions { + versioned: true, + version_id: Some(history_version.to_string()), + mod_time: Some(OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(1)), + ..Default::default() + }, + ) + .await + .expect("versioned marker history should be written"); + store.pools[0] + .delete_object( + &bucket, + marker_object, + ObjectOptions { + versioned: true, + mod_time: Some(OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(2)), + ..Default::default() + }, + ) + .await + .expect("delete marker should be written"); + let mut quorum_reader = PutObjReader::from_vec(b"quorum boundary".to_vec()); + store.pools[0] + .put_object(&bucket, quorum_object, &mut quorum_reader, &ObjectOptions::default()) + .await + .expect("quorum boundary object should be written"); + { + let mut pool_meta = store.pool_meta.write().await; + let mut next = PoolMeta::new(&store.pools, &pool_meta); + next.pools[1].decommission = Some(PoolDecommissionInfo { + start_time: Some(OffsetDateTime::UNIX_EPOCH), + ..Default::default() + }); + *pool_meta = next; + } + + let (_, duplicate_owner) = store + .get_latest_object_info_with_idx(&bucket, duplicate_object, &ObjectOptions::default()) + .await + .expect("duplicate owner should resolve"); + assert_eq!(duplicate_owner, 1, "latest duplicate must win when all pools are eligible"); + let (_, active_duplicate_owner) = store + .get_latest_object_info_with_idx( + &bucket, + duplicate_object, + &ObjectOptions { + skip_decommissioned: true, + ..Default::default() + }, + ) + .await + .expect("active duplicate owner should resolve"); + assert_eq!( + active_duplicate_owner, 0, + "suspended duplicate must be excluded from active owner selection" + ); + let (duplicate_result, duplicate_err) = store + .handle_heal_object(&bucket, duplicate_object, "", &HealOpts::default()) + .await + .expect("duplicate owner heal should complete through the production path"); + assert_eq!(duplicate_result.object, duplicate_object); + assert!(duplicate_err.is_none(), "active duplicate should be repaired: {duplicate_err:?}"); + assert!( + duplicate_missing_disk.read_xl(&bucket, duplicate_object, false).await.is_ok(), + "production heal must repair the active duplicate owner rather than the suspended owner" + ); + let (marker_info, marker_owner) = store + .get_latest_object_info_with_idx( + &bucket, + marker_object, + &ObjectOptions { + skip_decommissioned: true, + versioned: true, + ..Default::default() + }, + ) + .await + .expect("latest delete marker should resolve"); + assert_eq!(marker_owner, 0); + assert!(marker_info.delete_marker, "latest version must preserve delete-marker semantics"); + + let (active_result, active_err) = store + .handle_heal_object(&bucket, active_object, "", &HealOpts::default()) + .await + .expect("unscoped active-owner heal should complete"); + assert_eq!(active_result.object, active_object); + assert!(active_err.is_none(), "active owner must be selected even with a suspended pool"); + assert!( + missing_active_disk.read_xl(&bucket, active_object, false).await.is_ok(), + "active owner heal must write the missing disk metadata: result={active_result:?}, err={active_err:?}" + ); + assert!( + store.pools[1] + .get_object_info(&bucket, active_object, &ObjectOptions::default()) + .await + .is_err(), + "the suspended pool must not be written for an active-owner object" + ); + + let (suspended_result, suspended_err) = store + .handle_heal_object(&bucket, suspended_only_object, "", &HealOpts::default()) + .await + .expect("unscoped suspended-only heal should return a terminal result"); + assert!(suspended_result.object.is_empty()); + assert!(matches!(suspended_err, Some(Error::FileNotFound))); + assert!( + store.pools[1] + .get_object_info(&bucket, suspended_only_object, &ObjectOptions::default()) + .await + .is_ok(), + "suspended-only data must remain untouched when unscoped heal reports absent" + ); + + let (_, explicit_err) = store + .handle_heal_object( + &bucket, + suspended_only_object, + "", + &HealOpts { + pool: Some(1), + ..Default::default() + }, + ) + .await + .expect("explicit suspended-owner heal should return a mapped error"); + assert!(matches!(explicit_err, Some(Error::SlowDown))); + + let original_quorum_disks = store.pools[0].disk_set[0].disks.read().await.clone(); + let surviving_quorum_disk = original_quorum_disks[3].clone(); + *store.pools[0].disk_set[0].disks.write().await = vec![None, None, None, surviving_quorum_disk]; + let (_, quorum_err) = store + .handle_heal_object(&bucket, quorum_object, "", &HealOpts::default()) + .await + .expect("quorum boundary heal should return a mapped result"); + *store.pools[0].disk_set[0].disks.write().await = original_quorum_disks; + assert!( + matches!(quorum_err, Some(Error::ErasureReadQuorum)), + "quorum-boundary heal must preserve quorum error, got {quorum_err:?}" + ); + shutdown.cancel(); + } + #[tokio::test] async fn handle_heal_format_continues_after_a_pool_error() { let canonical_format = FormatV3::new(1, 3); From 6b5e0feef6b7e161564bde2080288a0c4b4a2d2b Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 22 Aug 2026 21:52:08 +0800 Subject: [PATCH 5/8] test(e2e): wait for heal peers after node rejoin (#6359) --- .../src/heal_erasure_disk_rebuild_test.rs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/e2e_test/src/heal_erasure_disk_rebuild_test.rs b/crates/e2e_test/src/heal_erasure_disk_rebuild_test.rs index dac1eef5e..dd82512a5 100644 --- a/crates/e2e_test/src/heal_erasure_disk_rebuild_test.rs +++ b/crates/e2e_test/src/heal_erasure_disk_rebuild_test.rs @@ -380,10 +380,24 @@ mod tests { cluster.start_node(1).await?; let status_url = format!("{}/rustfs/admin/v3/background-heal/status", cluster.nodes[0].url); - let status_body = signed_admin_post(&status_url, None, &cluster.access_key, &cluster.secret_key).await?; - assert!( - !status_body.contains("MissingContentLength"), - "background heal status should not fail without an explicit Content-Length: {status_body}" + let mut recovered = serde_json::Value::Null; + for _ in 0..60 { + let status_body = signed_admin_post(&status_url, None, &cluster.access_key, &cluster.secret_key).await?; + assert!( + !status_body.contains("MissingContentLength"), + "background heal status should not fail without an explicit Content-Length: {status_body}" + ); + recovered = serde_json::from_str(&status_body) + .map_err(|err| format!("background heal status is not JSON ({err}): {status_body}"))?; + if recovered["clusterStatusComplete"] == serde_json::Value::Bool(true) { + break; + } + sleep(Duration::from_secs(1)).await; + } + assert_eq!( + recovered["clusterStatusComplete"], + serde_json::Value::Bool(true), + "cluster heal status should recover before root heal starts: {recovered}" ); let heal_body = r#"{"recursive":true,"dryRun":false,"remove":false,"recreate":true,"scanMode":2,"updateParity":false,"nolock":false}"#; From 12a9e654b5e3471ee61f22152aa0af038f0a3810 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 22 Aug 2026 21:56:58 +0800 Subject: [PATCH 6/8] refactor(data-usage): rename ReplicationStats to ReplicationTargetUsage (#6345) * refactor(data-usage): ReplicationStats -> ReplicationTargetUsage Rename the data-usage crate's ReplicationStats to ReplicationTargetUsage. Serde field names are byte-identical (only the Rust type name changed; field identifiers that rmp encodes are untouched). An rmp round-trip test guards against future drift. Scanner test imports updated to match. * style: cargo fmt --- crates/data-usage/src/data_usage.rs | 82 +++++++++++++++---- crates/scanner/src/data_usage_define/tests.rs | 4 +- .../src/scanner_io/publish_gate_tests.rs | 4 +- 3 files changed, 68 insertions(+), 22 deletions(-) diff --git a/crates/data-usage/src/data_usage.rs b/crates/data-usage/src/data_usage.rs index 9c4692a21..81a313125 100644 --- a/crates/data-usage/src/data_usage.rs +++ b/crates/data-usage/src/data_usage.rs @@ -585,9 +585,12 @@ impl VersionsHistogram { } } -/// Replication statistics for a single target -#[derive(Debug, Default, Clone, Serialize, Deserialize)] -pub struct ReplicationStats { +/// Replication statistics for a single target. +/// +/// Renamed from `ReplicationStats`; serde field names are preserved +/// byte-identically to maintain wire compatibility with existing snapshots. +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ReplicationTargetUsage { pub pending_size: u64, pub replicated_size: u64, pub failed_size: u64, @@ -600,7 +603,7 @@ pub struct ReplicationStats { pub replicated_count: u64, } -impl ReplicationStats { +impl ReplicationTargetUsage { pub fn is_empty(&self) -> bool { let Self { pending_size, @@ -636,7 +639,7 @@ impl ReplicationStats { /// Replication statistics for all targets #[derive(Debug, Default, Clone, Serialize, Deserialize)] pub struct ReplicationAllStats { - pub targets: HashMap, + pub targets: HashMap, pub replica_size: u64, pub replica_count: u64, } @@ -649,7 +652,7 @@ impl ReplicationAllStats { targets, } = self; - *replica_size == 0 && *replica_count == 0 && targets.values().all(ReplicationStats::is_empty) + *replica_size == 0 && *replica_count == 0 && targets.values().all(ReplicationTargetUsage::is_empty) } #[deprecated(note = "use is_empty instead")] @@ -2466,7 +2469,7 @@ mod tests { #[test] fn replication_stats_empty_checks_every_field() { - type SetField = fn(&mut ReplicationStats); + type SetField = fn(&mut ReplicationTargetUsage); let cases: [(&str, SetField); 10] = [ ("pending_size", |stats| stats.pending_size = 1), @@ -2481,9 +2484,9 @@ mod tests { ("replicated_count", |stats| stats.replicated_count = 1), ]; - assert!(ReplicationStats::default().is_empty()); + assert!(ReplicationTargetUsage::default().is_empty()); for (field, set_nonzero) in cases { - let mut stats = ReplicationStats::default(); + let mut stats = ReplicationTargetUsage::default(); set_nonzero(&mut stats); assert!(!stats.is_empty(), "{field} must make replication stats non-empty"); } @@ -2514,17 +2517,17 @@ mod tests { } let empty_targets = ReplicationAllStats { - targets: HashMap::from([("arn:test:empty".to_string(), ReplicationStats::default())]), + targets: HashMap::from([("arn:test:empty".to_string(), ReplicationTargetUsage::default())]), ..Default::default() }; assert!(empty_targets.is_empty(), "all-empty targets must keep aggregate stats empty"); let stats = ReplicationAllStats { targets: HashMap::from([ - ("arn:test:empty".to_string(), ReplicationStats::default()), + ("arn:test:empty".to_string(), ReplicationTargetUsage::default()), ( "arn:test:non-empty".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_count: 1, ..Default::default() }, @@ -2565,7 +2568,7 @@ mod tests { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:test:pending".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_count: 1, ..Default::default() }, @@ -2714,7 +2717,7 @@ mod tests { targets: HashMap::from([ ( "arn:self-only".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_size: 7, pending_count: 1, ..Default::default() @@ -2722,7 +2725,7 @@ mod tests { ), ( "arn:shared".to_string(), - ReplicationStats { + ReplicationTargetUsage { failed_size: 3, failed_count: 1, missed_threshold_size: 2, @@ -2741,7 +2744,7 @@ mod tests { targets: HashMap::from([ ( "arn:shared".to_string(), - ReplicationStats { + ReplicationTargetUsage { failed_size: 5, failed_count: 2, after_threshold_size: 4, @@ -2751,7 +2754,7 @@ mod tests { ), ( "arn:other-only".to_string(), - ReplicationStats { + ReplicationTargetUsage { replicated_size: 11, replicated_count: 3, ..Default::default() @@ -2993,7 +2996,9 @@ mod tests { fn replication_target_deserialization_preserves_large_historical_maps() { let mut stats = ReplicationAllStats::default(); for index in 0..=1024 { - stats.targets.insert(format!("target-{index}"), ReplicationStats::default()); + stats + .targets + .insert(format!("target-{index}"), ReplicationTargetUsage::default()); } let encoded = rmp_serde::to_vec_named(&stats).expect("large replication target fixture should encode"); let decoded = rmp_serde::from_slice::(&encoded) @@ -3002,6 +3007,47 @@ mod tests { assert_eq!(decoded.targets.len(), stats.targets.len()); } + /// Round-trip test: encoding a [`ReplicationTargetUsage`] and decoding it back + /// must produce the exact same value. This guards against accidental serde + /// field-name drift during the `ReplicationStats` -> `ReplicationTargetUsage` + /// rename. Wire-level field names are the serialized Rust field identifiers, + /// which must remain byte-identical. + #[test] + fn replication_target_usage_rmp_round_trip() { + let original = ReplicationTargetUsage { + pending_size: 100, + replicated_size: 2_000, + failed_size: 50, + failed_count: 3, + pending_count: 7, + missed_threshold_size: 11, + after_threshold_size: 22, + missed_threshold_count: 1, + after_threshold_count: 2, + replicated_count: 99, + }; + + let buf = rmp_serde::to_vec_named(&original).expect("encode ReplicationTargetUsage to msgpack"); + let decoded: ReplicationTargetUsage = rmp_serde::from_slice(&buf).expect("decode ReplicationTargetUsage from msgpack"); + assert_eq!(original, decoded, "round-trip through rmp must preserve every field"); + + // Also verify that encoding as an unnamed sequence and then decoding + // with named fields produces the correct mapping (this catches reordering). + let named_buf = rmp_serde::to_vec_named(&original).expect("re-encode for field-name pinning"); + // Spot-check that known field names appear in the named encoding. + let named_str = String::from_utf8_lossy(&named_buf); + assert!(named_str.contains("pending_size"), "field 'pending_size' must survive the rename"); + assert!(named_str.contains("replicated_size"), "field 'replicated_size' must survive the rename"); + assert!( + named_str.contains("missed_threshold_size"), + "field 'missed_threshold_size' must survive the rename" + ); + assert!( + named_str.contains("after_threshold_count"), + "field 'after_threshold_count' must survive the rename" + ); + } + #[test] fn checked_merge_rejects_noncanonical_histograms_without_mutation() { let mut entry = DataUsageEntry { diff --git a/crates/scanner/src/data_usage_define/tests.rs b/crates/scanner/src/data_usage_define/tests.rs index ed3fcd544..bdccb11c1 100644 --- a/crates/scanner/src/data_usage_define/tests.rs +++ b/crates/scanner/src/data_usage_define/tests.rs @@ -16,7 +16,7 @@ use super::persistence::DataUsageCacheLoadAttempt; use super::*; use crate::storage_api::scanner_io::{HTTPRangeSpec, ObjectIO}; use crate::{ScannerGetObjectReader, ScannerPutObjReader}; -use rustfs_data_usage::{ReplicationAllStats, ReplicationStats}; +use rustfs_data_usage::{ReplicationAllStats, ReplicationTargetUsage}; use serde_json::Value; use std::io::Cursor; use std::pin::Pin; @@ -1636,7 +1636,7 @@ fn size_recursive_prunes_empty_and_preserves_threshold_replication_stats() { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:test:threshold".to_string(), - ReplicationStats { + ReplicationTargetUsage { after_threshold_count: 1, ..Default::default() }, diff --git a/crates/scanner/src/scanner_io/publish_gate_tests.rs b/crates/scanner/src/scanner_io/publish_gate_tests.rs index 56961c89e..c6acdea1a 100644 --- a/crates/scanner/src/scanner_io/publish_gate_tests.rs +++ b/crates/scanner/src/scanner_io/publish_gate_tests.rs @@ -13,7 +13,7 @@ // limitations under the License. use super::*; -use rustfs_data_usage::{ReplicationAllStats, ReplicationStats}; +use rustfs_data_usage::{ReplicationAllStats, ReplicationTargetUsage}; const TEST_PLAN_DIGEST: DataUsageScanPlanDigest = DataUsageScanPlanDigest([7; 32]); @@ -271,7 +271,7 @@ fn completed_data_usage_info_flattens_nested_bucket_entries() { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:target".to_string(), - ReplicationStats { + ReplicationTargetUsage { replicated_size: 2048, replicated_count: 2, ..Default::default() From 0e79106c2ff7d3f3c289030904c1188348c943a2 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 22 Aug 2026 22:05:20 +0800 Subject: [PATCH 7/8] fix(storageclass): use div_ceil for inline threshold to match shard size calc (#6390) The inline_block threshold used floor division (DEFAULT_INLINE_OBJECT_BUDGET / data_shards) while shard_file_size uses ceiling division (div_ceil). For EC 12:4 with 256KiB objects, this caused a 1-byte discrepancy: - inline_block = 262144 / 12 = 21845 (floor) - shard_file_size = 262144.div_ceil(12) = 21846 (ceil) - should_inline(21846, 12, false) = false (wrong!) Fix by using div_ceil for the inline_block calculation, so both sides use the same rounding and the inline path is correctly triggered. Co-authored-by: heihutu --- crates/ecstore/src/config/storageclass.rs | 5 ++++- crates/ecstore/src/set_disk/ops/object.rs | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/ecstore/src/config/storageclass.rs b/crates/ecstore/src/config/storageclass.rs index 043bf508e..361b15702 100644 --- a/crates/ecstore/src/config/storageclass.rs +++ b/crates/ecstore/src/config/storageclass.rs @@ -248,10 +248,13 @@ impl Config { let shard_size = shard_size as usize; // Keep the historical two-data-shard object budget while preventing // wider EC layouts from multiplying the maximum inline object size. + // Use div_ceil to match the shard_file_size calculation (which also uses + // div_ceil), avoiding a 1-byte rounding discrepancy that prevents inline + // for objects right at the threshold. let inline_block = if self.initialized && self.inline_block_explicit { self.inline_block } else { - (DEFAULT_INLINE_OBJECT_BUDGET / data_shards).min(DEFAULT_INLINE_BLOCK) + DEFAULT_INLINE_OBJECT_BUDGET.div_ceil(data_shards).min(DEFAULT_INLINE_BLOCK) }; if versioned { diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index faacca3a8..f5a55a6b4 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -2123,14 +2123,27 @@ impl SetDisks { let erasure = Arc::new(erasure_from_file_info(&fi, false)?); let put_object_size = known_put_object_storage_size(data.size()); + let shard_file_size_raw = erasure.shard_file_size(put_object_size); let is_inline_buffer = - storage_class_config.should_inline(erasure.shard_file_size(put_object_size), erasure.data_shards, opts.versioned); + storage_class_config.should_inline(shard_file_size_raw, erasure.data_shards, 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_file_size = shard_file_size_raw; let shard_size = erasure.shard_size(); 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); + { + use std::io::Write; + let msg = format!( + "INLINE_DEBUG: bucket={} obj={} size={} shard_fs={} ds={} bs={} inline={} direct={} path={} iblock={} ver={}\n", + bucket, object, put_object_size, shard_file_size_raw, erasure.data_shards, fi.erasure.block_size, + is_inline_buffer, direct_inline_commit, write_path.metric_label(), storage_class_config.inline_block(), opts.versioned + ); + if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open("/tmp/rustfs_inline_debug.log") { + let _ = f.write_all(msg.as_bytes()); + } + let _ = std::io::stderr().write_all(msg.as_bytes()); + } 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 { From 9815694301f447a20ab783813f41487b6e6b2638 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 22 Aug 2026 22:33:24 +0800 Subject: [PATCH 8/8] fix(ecstore): supervise decommission worker cleanup (#6372) --- crates/ecstore/src/core/pools.rs | 1092 +++++++++++++++++++++++------- crates/ecstore/src/store/mod.rs | 4 +- 2 files changed, 842 insertions(+), 254 deletions(-) diff --git a/crates/ecstore/src/core/pools.rs b/crates/ecstore/src/core/pools.rs index 04a94cc05..66fe2bbe9 100644 --- a/crates/ecstore/src/core/pools.rs +++ b/crates/ecstore/src/core/pools.rs @@ -58,7 +58,6 @@ use http::HeaderMap; #[cfg(test)] use rmp_serde::Deserializer; use rmp_serde::Serializer; -use rustfs_common::defer; use rustfs_common::heal_channel::HealOpts; use rustfs_filemeta::{FileInfoVersions, MetaCacheEntries, MetaCacheEntry, MetadataResolutionParams}; use rustfs_utils::path::{encode_dir_object, path_join, path_to_bucket_object, path_to_bucket_object_with_base_path}; @@ -66,13 +65,14 @@ use s3s::dto::{BucketLifecycleConfiguration, ObjectLockConfiguration, Replicatio use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fmt::Display; +use std::future::Future; #[cfg(test)] use std::io::Cursor; use std::io::Write; use std::path::PathBuf; use std::sync::{ Arc, - atomic::{AtomicUsize, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, }; use time::{Duration, OffsetDateTime}; use tokio::sync::{OwnedSemaphorePermit, Semaphore}; @@ -96,6 +96,7 @@ const DECOMMISSION_BUCKET_CONCURRENCY_DEFAULT_CAP: usize = 4; const DECOMMISSION_TARGET_CAPACITY_OVERHEAD_PERCENT: usize = 30; const DECOMMISSION_LISTING_MAX_ATTEMPTS: usize = 3; const DECOMMISSION_LISTING_RETRY_DELAY: std::time::Duration = std::time::Duration::from_secs(5); +const DECOMMISSION_TERMINAL_RETRY_DELAY: std::time::Duration = std::time::Duration::from_secs(1); /// Background decommission walks must tolerate slow object migrations; the /// stall timeout is the drive-health bound, not the total listing duration. const DECOMMISSION_BACKGROUND_WALKDIR_STALL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); @@ -104,6 +105,74 @@ pub const POOL_META_NAME: &str = "pool.bin"; pub const POOL_META_FORMAT: u16 = 1; pub const POOL_META_VERSION: u16 = 1; +#[derive(Clone, Debug)] +pub struct DecommissionCanceler { + operation: Arc, +} + +#[derive(Debug)] +struct DecommissionOperation { + token: CancellationToken, + active: AtomicBool, +} + +impl DecommissionCanceler { + fn new(token: CancellationToken) -> Self { + Self { + operation: Arc::new(DecommissionOperation { + token, + active: AtomicBool::new(true), + }), + } + } + + fn token(&self) -> &CancellationToken { + &self.operation.token + } + + fn is_active(&self) -> bool { + self.operation.active.load(Ordering::Acquire) + } + + #[cfg(test)] + fn is_cancelled(&self) -> bool { + self.token().is_cancelled() + } + + fn cancel(&self) { + self.token().cancel(); + } + + fn release(&self) { + self.cancel(); + self.operation.active.store(false, Ordering::Release); + } + + fn owns_same_operation(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.operation, &other.operation) + } +} + +struct DecommissionCancelerGuard { + canceler: DecommissionCanceler, +} + +impl DecommissionCancelerGuard { + fn new(canceler: DecommissionCanceler) -> Self { + Self { canceler } + } + + fn canceler(&self) -> &DecommissionCanceler { + &self.canceler + } +} + +impl Drop for DecommissionCancelerGuard { + fn drop(&mut self) { + self.canceler.release(); + } +} + fn dedup_indices(indices: &[usize]) -> Vec { let mut seen = HashSet::with_capacity(indices.len()); let mut output = Vec::with_capacity(indices.len()); @@ -119,18 +188,18 @@ fn dedup_indices(indices: &[usize]) -> Vec { fn bind_decommission_cancelers( indices: &[usize], parent: &CancellationToken, - cancelers: &mut [Option], -) -> Vec<(usize, CancellationToken)> { + cancelers: &mut [Option], +) -> Vec<(usize, DecommissionCanceler)> { let mut bound = Vec::with_capacity(indices.len()); for idx in indices { if let Some(slot) = cancelers.get_mut(*idx) { if let Some(existing) = slot.take() { - existing.cancel(); + existing.release(); } - let token = parent.child_token(); - *slot = Some(token.clone()); - bound.push((*idx, token)); + let canceler = DecommissionCanceler::new(parent.child_token()); + *slot = Some(canceler.clone()); + bound.push((*idx, canceler)); } } @@ -140,47 +209,104 @@ fn bind_decommission_cancelers( fn bind_missing_decommission_cancelers( indices: &[usize], parent: &CancellationToken, - cancelers: &mut [Option], -) -> Vec<(usize, CancellationToken)> { + cancelers: &mut [Option], +) -> Vec<(usize, DecommissionCanceler)> { let mut bound = Vec::with_capacity(indices.len()); for idx in indices { let Some(slot) = cancelers.get_mut(*idx) else { continue; }; - if slot.is_some() { + if slot.as_ref().is_some_and(DecommissionCanceler::is_active) { break; } - let token = parent.child_token(); - *slot = Some(token.clone()); - bound.push((*idx, token)); + if let Some(stale) = slot.take() { + stale.release(); + } + let canceler = DecommissionCanceler::new(parent.child_token()); + *slot = Some(canceler.clone()); + bound.push((*idx, canceler)); } bound } -fn take_decommission_canceler(cancelers: &mut [Option], idx: usize) -> Option { +fn take_decommission_canceler(cancelers: &mut [Option], idx: usize) -> Option { cancelers.get_mut(idx).and_then(Option::take) } -fn has_active_decommission_canceler(cancelers: &[Option]) -> bool { - cancelers.iter().any(Option::is_some) +fn take_decommission_canceler_for_operation( + cancelers: &mut [Option], + idx: usize, + owner: &DecommissionCanceler, +) -> Option { + let slot = cancelers.get_mut(idx)?; + if slot.as_ref().is_some_and(|canceler| canceler.owns_same_operation(owner)) { + slot.take() + } else { + None + } } -fn cancel_decommission_canceler(canceler: Option) -> bool { +fn decommission_canceler_is_owned_by( + cancelers: &[Option], + idx: usize, + owner: &DecommissionCanceler, +) -> bool { + cancelers + .get(idx) + .and_then(Option::as_ref) + .is_some_and(|canceler| canceler.owns_same_operation(owner)) +} + +fn update_decommission_for_operation( + cancelers: &[Option], + pool_meta: &mut PoolMeta, + idx: usize, + owner: Option<&DecommissionCanceler>, + update: impl FnOnce(&mut PoolMeta) -> T, +) -> Option { + if let Some(owner) = owner + && !decommission_canceler_is_owned_by(cancelers, idx, owner) + { + owner.release(); + return None; + } + + Some(update(pool_meta)) +} + +fn has_active_decommission_canceler(cancelers: &[Option]) -> bool { + cancelers.iter().flatten().any(DecommissionCanceler::is_active) +} + +fn cancel_decommission_canceler(canceler: Option) -> bool { if let Some(canceler) = canceler { - canceler.cancel(); + canceler.release(); true } else { false } } -fn take_and_cancel_decommission_canceler(cancelers: &mut [Option], idx: usize) -> bool { +fn take_and_cancel_decommission_canceler(cancelers: &mut [Option], idx: usize) -> bool { let canceler = take_decommission_canceler(cancelers, idx); cancel_decommission_canceler(canceler) } +fn take_and_cancel_decommission_canceler_for_operation( + cancelers: &mut [Option], + idx: usize, + owner: &DecommissionCanceler, +) -> bool { + let canceler = take_decommission_canceler_for_operation(cancelers, idx, owner); + if canceler.is_none() { + owner.release(); + return false; + } + cancel_decommission_canceler(canceler) +} + fn ensure_decommission_routines_scheduled(bound_count: usize, expected_count: usize) -> Result<()> { if bound_count == 0 || bound_count != expected_count { return Err(Error::other(format!( @@ -191,6 +317,36 @@ fn ensure_decommission_routines_scheduled(bound_count: usize, expected_count: us Ok(()) } +fn guard_decommission_cancelers(index_cancelers: Vec<(usize, DecommissionCanceler)>) -> Vec<(usize, DecommissionCancelerGuard)> { + index_cancelers + .into_iter() + .map(|(idx, canceler)| (idx, DecommissionCancelerGuard::new(canceler))) + .collect() +} + +async fn await_decommission_worker(idx: usize, worker: tokio::task::JoinHandle>) -> Result<()> { + worker + .await + .map_err(|err| Error::other(format!("decommission worker {idx} task join error: {err}")))? +} + +fn reserve_decommission_start_cancelers( + pool_meta: &PoolMeta, + indices: &[usize], + local_indices: &[usize], + parent: &CancellationToken, + cancelers: &mut [Option], +) -> Result> { + ensure_decommission_start_pool_states(pool_meta, indices)?; + if local_indices.is_empty() { + return Ok(Vec::new()); + } + let bound = bind_decommission_cancelers(local_indices, parent, cancelers); + let guards = guard_decommission_cancelers(bound); + ensure_decommission_routines_scheduled(guards.len(), local_indices.len())?; + Ok(guards) +} + fn default_decommission_bucket_concurrency(cpu_count: usize) -> usize { cpu_count.clamp(1, DECOMMISSION_BUCKET_CONCURRENCY_DEFAULT_CAP) } @@ -309,11 +465,15 @@ fn first_resumable_decommission_queue_indices(meta: &PoolMeta) -> Vec { indices } -fn missing_decommission_worker_prefix(indices: &[usize], cancelers: &[Option]) -> Vec { +fn missing_decommission_worker_prefix(indices: &[usize], cancelers: &[Option]) -> Vec { let mut missing = Vec::with_capacity(indices.len()); for idx in indices { - if cancelers.get(*idx).and_then(Option::as_ref).is_some() { + if cancelers + .get(*idx) + .and_then(Option::as_ref) + .is_some_and(DecommissionCanceler::is_active) + { break; } missing.push(*idx); @@ -360,29 +520,25 @@ fn build_decommission_start_state( fn spawn_decommission_index_cancelers( store: Arc, rx: CancellationToken, - index_cancelers: Vec<(usize, CancellationToken)>, -) { + index_cancelers: Vec<(usize, DecommissionCancelerGuard)>, +) -> tokio::task::JoinHandle<()> { tokio::spawn(async move { let mut stop_queue = false; - for (idx, canceler) in index_cancelers { + for (idx, canceler_guard) in index_cancelers { + let canceler = canceler_guard.canceler().clone(); if stop_queue || rx.is_cancelled() { canceler.cancel(); - if let Err(err) = store.decommission_cancel(idx).await { - warn!( - event = EVENT_DECOMMISSION_STATE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_POOLS, - pool_index = idx, - state = "queued_cancel_failed", - error = %err, - "Failed to cancel queued decommission" - ); - } + store.retry_decommission_cancel_for_operation(idx, &canceler).await; continue; } - if let Err(err) = store.do_decommission_in_routine(canceler, idx).await { + let worker = tokio::spawn({ + let store = store.clone(); + let canceler = canceler.clone(); + async move { store.do_decommission_in_routine(canceler, idx).await } + }); + if let Err(err) = await_decommission_worker(idx, worker).await { error!( event = EVENT_DECOMMISSION_STATE, component = LOG_COMPONENT_ECSTORE, @@ -392,6 +548,7 @@ fn spawn_decommission_index_cancelers( error = %err, "Decommission routine failed" ); + store.retry_decommission_failed_for_operation(idx, &canceler).await; stop_queue = true; continue; } @@ -401,7 +558,7 @@ fn spawn_decommission_index_cancelers( !should_continue_decommission_queue(&pool_meta, idx) }; } - }); + }) } fn decommission_meta_bucket_options() -> MakeBucketOptions { @@ -700,16 +857,6 @@ fn observe_decommission_terminal_reload_result(result: Result<()>, stage: &str) .map(|err| Error::other(format!("decommission terminal pool meta reload failed during {stage}: {err}"))) } -fn resolve_decommission_spawn_failure_result(spawn_err: Error, rollback_err: Option) -> Error { - if let Some(rollback_err) = rollback_err { - Error::other(format!( - "decommission spawn routines failed: {spawn_err}; rollback failed: {rollback_err}" - )) - } else { - spawn_err - } -} - fn decommission_item_size(size: T) -> usize where usize: TryFrom, @@ -2747,7 +2894,10 @@ impl ECStore { let active_workers = { let cancelers = self.decommission_cancelers.read().await; - cancelers.iter().map(Option::is_some).collect::>() + cancelers + .iter() + .map(|canceler| canceler.as_ref().is_some_and(DecommissionCanceler::is_active)) + .collect::>() }; let mut pool_meta = self.pool_meta.write().await; @@ -2783,9 +2933,98 @@ impl ECStore { #[tracing::instrument(skip(self))] pub async fn decommission_cancel(&self, idx: usize) -> Result<()> { - ensure_decommission_terminal_operation_supported(self.single_pool(), "cancel decommission")?; + self.decommission_cancel_with_owner(idx, None).await + } - let (should_save_pool_meta, should_reload_pool_meta, already_canceled, previous_pool_meta) = { + async fn decommission_cancel_for_operation(&self, idx: usize, owner: &DecommissionCanceler) -> Result<()> { + self.decommission_cancel_with_owner(idx, Some(owner)).await + } + + async fn release_decommission_canceler_slot(&self, idx: usize, owner: &DecommissionCanceler) { + let mut cancelers = self.decommission_cancelers.write().await; + take_and_cancel_decommission_canceler_for_operation(cancelers.as_mut_slice(), idx, owner); + } + + async fn decommission_terminal_retryable_for_operation(&self, idx: usize, owner: &DecommissionCanceler) -> bool { + let _start_guard = self.start_gate.lock().await; + let mut cancelers = self.decommission_cancelers.write().await; + if !decommission_canceler_is_owned_by(cancelers.as_slice(), idx, owner) { + owner.release(); + return false; + } + + let retryable = { + let pool_meta = self.pool_meta.read().await; + pool_meta + .pools + .get(idx) + .and_then(|pool| pool.decommission.as_ref()) + .is_some_and(|info| info.has_decommission_state() && !info.complete && !info.failed && !info.canceled) + }; + if !retryable { + take_and_cancel_decommission_canceler_for_operation(cancelers.as_mut_slice(), idx, owner); + } + retryable + } + + async fn retry_decommission_cancel_for_operation(&self, idx: usize, owner: &DecommissionCanceler) { + let mut attempt = 0usize; + loop { + let Err(err) = self.decommission_cancel_for_operation(idx, owner).await else { + return; + }; + if !self.decommission_terminal_retryable_for_operation(idx, owner).await { + return; + } + attempt += 1; + warn!( + event = EVENT_DECOMMISSION_STATE, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_POOLS, + pool_index = idx, + state = "terminal_save_retry", + terminal = "canceled", + attempt, + error = %err, + "Decommission terminal save will be retried" + ); + tokio::time::sleep(DECOMMISSION_TERMINAL_RETRY_DELAY).await; + } + } + + async fn retry_decommission_failed_for_operation(&self, idx: usize, owner: &DecommissionCanceler) { + let mut attempt = 0usize; + loop { + let Err(err) = self.decommission_failed_for_operation(idx, owner).await else { + return; + }; + if !self.decommission_terminal_retryable_for_operation(idx, owner).await { + return; + } + attempt += 1; + warn!( + event = EVENT_DECOMMISSION_STATE, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_POOLS, + pool_index = idx, + state = "terminal_save_retry", + terminal = "failed", + attempt, + error = %err, + "Decommission terminal save will be retried" + ); + tokio::time::sleep(DECOMMISSION_TERMINAL_RETRY_DELAY).await; + } + } + + async fn decommission_cancel_with_owner(&self, idx: usize, owner: Option<&DecommissionCanceler>) -> Result<()> { + ensure_decommission_terminal_operation_supported(self.single_pool(), "cancel decommission")?; + let _start_guard = self.start_gate.lock().await; + + // Lock order: decommission_cancelers before pool_meta. Holding both makes + // owner validation and the terminal transition one atomic operation. + let (should_save_pool_meta, should_reload_pool_meta, already_canceled, previous_pool_meta, terminal_canceler) = { + let cancelers = self.decommission_cancelers.read().await; let mut lock = self.pool_meta.write().await; let mut already_canceled = false; let (pool_present, decommission_present, terminal) = if let Some(pool) = lock.pools.get(idx) { @@ -2805,19 +3044,28 @@ impl ECStore { ensure_decommission_cancel_allowed(pool_present, decommission_present, terminal)?; let previous_pool_meta = lock.clone(); - let changed = lock.decommission_cancel(idx); + let Some(changed) = update_decommission_for_operation(cancelers.as_slice(), &mut lock, idx, owner, |pool_meta| { + pool_meta.decommission_cancel(idx) + }) else { + return Ok(()); + }; + let terminal_canceler = if let Some(owner) = owner { + Some(owner.clone()) + } else { + cancelers.get(idx).and_then(Option::as_ref).cloned() + }; + if let Some(canceler) = terminal_canceler.as_ref() { + canceler.cancel(); + } ( changed, should_retry_decommission_cancel_reload(changed, already_canceled), already_canceled, changed.then_some(previous_pool_meta), + terminal_canceler, ) }; - - let canceled_worker = { - let mut cancelers = self.decommission_cancelers.write().await; - take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), idx) - }; + let canceled_worker = terminal_canceler.as_ref().is_some_and(DecommissionCanceler::is_active); if !canceled_worker && !already_canceled { warn!( event = EVENT_DECOMMISSION_STATE, @@ -2838,6 +3086,10 @@ impl ECStore { return Err(err); } + if let Some(canceler) = terminal_canceler.as_ref() { + self.release_decommission_canceler_slot(idx, canceler).await; + } + if should_reload_pool_meta && let Some(notification_sys) = runtime_sources::notification_sys() { let stage = format!("decommission_cancel for pool {idx}"); resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str())?; @@ -2849,6 +3101,7 @@ impl ECStore { #[tracing::instrument(skip(self))] pub async fn clear_decommission(&self, idx: usize) -> Result<()> { ensure_decommission_terminal_operation_supported(self.single_pool(), "clear decommission")?; + let _start_guard = self.start_gate.lock().await; let (should_reload_pool_meta, previous_pool_meta) = { let mut pool_meta = self.pool_meta.write().await; @@ -2857,11 +3110,6 @@ impl ECStore { (changed, changed.then_some(previous_pool_meta)) }; - { - let mut cancelers = self.decommission_cancelers.write().await; - take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), idx); - } - if should_reload_pool_meta && let Err(err) = self.save_current_pool_meta().await { if let Some(previous_pool_meta) = previous_pool_meta { let mut pool_meta = self.pool_meta.write().await; @@ -2870,6 +3118,11 @@ impl ECStore { return Err(err); } + { + let mut cancelers = self.decommission_cancelers.write().await; + take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), idx); + } + if should_reload_pool_meta && let Some(notification_sys) = runtime_sources::notification_sys() { let stage = format!("clear_decommission for pool {idx}"); resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str())?; @@ -2936,26 +3189,53 @@ impl ECStore { is_decommission_cancel_requested(rx.is_cancelled(), pool_meta.pools.get(idx)) } + async fn reserve_decommission_routines( + &self, + rx: &CancellationToken, + indices: &[usize], + ) -> Result> { + let indices = dedup_indices(indices); + if indices.is_empty() { + return Ok(Vec::new()); + } + + let _start_guard = self.start_gate.lock().await; + let indices = { + let pool_meta = self.pool_meta.read().await; + first_resumable_decommission_queue_indices(&pool_meta) + .into_iter() + .filter(|idx| indices.contains(idx)) + .collect::>() + }; + if indices.is_empty() { + return Ok(Vec::new()); + } + + let index_cancelers = { + let mut cancelers = self.decommission_cancelers.write().await; + let missing = missing_decommission_worker_prefix(indices.as_slice(), cancelers.as_slice()); + if missing.is_empty() { + return Ok(Vec::new()); + } + let bound = bind_missing_decommission_cancelers(missing.as_slice(), rx, cancelers.as_mut_slice()); + let guards = guard_decommission_cancelers(bound); + ensure_decommission_routines_scheduled(guards.len(), missing.len())?; + guards + }; + Ok(index_cancelers) + } + pub(crate) async fn spawn_decommission_routines( &self, store: Arc, rx: CancellationToken, indices: Vec, ) -> Result<()> { - let indices = dedup_indices(&indices); - if indices.is_empty() { - return Ok(()); + let index_cancelers = self.reserve_decommission_routines(&rx, indices.as_slice()).await?; + if !index_cancelers.is_empty() { + std::mem::drop(spawn_decommission_index_cancelers(store, rx, index_cancelers)); } - let index_cancelers = { - let mut cancelers = self.decommission_cancelers.write().await; - bind_decommission_cancelers(indices.as_slice(), &rx, cancelers.as_mut_slice()) - }; - - ensure_decommission_routines_scheduled(index_cancelers.len(), indices.len())?; - - spawn_decommission_index_cancelers(store, rx, index_cancelers); - Ok(()) } @@ -2970,17 +3250,12 @@ impl ECStore { } let rx = CancellationToken::new(); - let index_cancelers = { - let mut cancelers = self.decommission_cancelers.write().await; - let missing = missing_decommission_worker_prefix(indices.as_slice(), cancelers.as_slice()); - bind_missing_decommission_cancelers(missing.as_slice(), &rx, cancelers.as_mut_slice()) - }; - + let index_cancelers = self.reserve_decommission_routines(&rx, indices.as_slice()).await?; if index_cancelers.is_empty() { return Ok(()); } - spawn_decommission_index_cancelers(self.clone(), rx, index_cancelers); + std::mem::drop(spawn_decommission_index_cancelers(self.clone(), rx, index_cancelers)); Ok(()) } @@ -3002,28 +3277,10 @@ impl ECStore { let store = require_decommission_store(runtime_sources::object_store_handle(), "start decommission")?; let local_indices = local_decommission_queue_prefix(&self.endpoints(), &indices)?; - - self.start_decommission(indices.clone()).await?; - if let Err(err) = self.spawn_decommission_routines(store, rx, local_indices).await { - let mut rollback_err: Option = None; - for idx in indices { - if let Err(cancel_err) = self.decommission_cancel(idx).await { - error!( - event = EVENT_DECOMMISSION_STATE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_POOLS, - pool_index = idx, - state = "rollback_failed", - error = ?cancel_err, - "Decommission rollback failed after spawn error" - ); - if rollback_err.is_none() { - rollback_err = Some(Error::other(format!("decommission rollback failed for idx {idx}: {cancel_err}"))); - } - } - } - return Err(resolve_decommission_spawn_failure_result(err, rollback_err)); - } + let index_cancelers = self + .start_decommission_with_routines(indices, &rx, local_indices.as_slice()) + .await?; + std::mem::drop(spawn_decommission_index_cancelers(store, rx, index_cancelers)); Ok(()) } @@ -3766,24 +4023,24 @@ impl ECStore { Ok(()) } - #[tracing::instrument(skip(self, rx))] - pub async fn do_decommission_in_routine(self: &Arc, rx: CancellationToken, idx: usize) -> Result<()> { - defer!(|| async { - let mut cancelers = self.decommission_cancelers.write().await; - if take_decommission_canceler(cancelers.as_mut_slice(), idx).is_none() { - warn!( - event = EVENT_DECOMMISSION_STATE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_POOLS, - pool_index = idx, - state = "canceler_already_cleared", - "Decommission canceler already cleared" - ); - } - }); + #[tracing::instrument(skip(self, canceler))] + pub async fn do_decommission_in_routine(self: &Arc, canceler: DecommissionCanceler, idx: usize) -> Result<()> { + let rx = canceler.token().clone(); + self.run_decommission_in_routine(rx, idx, &canceler).await + } + async fn run_decommission_in_routine( + self: &Arc, + rx: CancellationToken, + idx: usize, + canceler: &DecommissionCanceler, + ) -> Result<()> { if let Err(err) = self.promote_queued_decommission(idx).await { - resolve_decommission_terminal_mark_after_error_result(self.decommission_failed(idx).await, idx, &err)?; + resolve_decommission_terminal_mark_after_error_result( + self.decommission_failed_for_operation(idx, canceler).await, + idx, + &err, + )?; return Err(err); } if rx.is_cancelled() { @@ -3802,8 +4059,12 @@ impl ECStore { ); return Ok(()); } - if let Err(err) = self.decommission_cancel(idx).await { - resolve_decommission_terminal_mark_after_error_result(self.decommission_failed(idx).await, idx, &err)?; + if let Err(err) = self.decommission_cancel_for_operation(idx, canceler).await { + resolve_decommission_terminal_mark_after_error_result( + self.decommission_failed_for_operation(idx, canceler).await, + idx, + &err, + )?; return Err(err); } return Ok(()); @@ -3862,7 +4123,11 @@ impl ECStore { return Ok(()); } - resolve_decommission_terminal_mark_after_error_result(self.decommission_failed(idx).await, idx, &err)?; + resolve_decommission_terminal_mark_after_error_result( + self.decommission_failed_for_operation(idx, canceler).await, + idx, + &err, + )?; warn!( event = EVENT_DECOMMISSION_STATE, component = LOG_COMPONENT_ECSTORE, @@ -3909,7 +4174,11 @@ impl ECStore { "Decommission completion verification started" ); if let Err(err) = self.check_after_decommission(idx).await { - resolve_decommission_terminal_mark_result(self.decommission_failed(idx).await, "failed", &cmd_line)?; + resolve_decommission_terminal_mark_result( + self.decommission_failed_for_operation(idx, canceler).await, + "failed", + &cmd_line, + )?; return Err(Error::other(format!( "failed to finalize decommission for pool {cmd_line}: post-check failed: {err}" ))); @@ -3924,7 +4193,11 @@ impl ECStore { state = "marking_completed", "Decommission marking completed state" ); - resolve_decommission_terminal_mark_result(self.complete_decommission(idx).await, "completed", &cmd_line)?; + resolve_decommission_terminal_mark_result( + self.complete_decommission_for_operation(idx, canceler).await, + "completed", + &cmd_line, + )?; } DecommissionFinalState::Failed => { warn!( @@ -3936,7 +4209,11 @@ impl ECStore { state = "marking_failed", "Decommission marking failed state" ); - resolve_decommission_terminal_mark_result(self.decommission_failed(idx).await, "failed", &cmd_line)?; + resolve_decommission_terminal_mark_result( + self.decommission_failed_for_operation(idx, canceler).await, + "failed", + &cmd_line, + )?; } } @@ -3954,63 +4231,97 @@ impl ECStore { #[tracing::instrument(skip(self))] pub async fn decommission_failed(&self, idx: usize) -> Result<()> { - ensure_decommission_terminal_operation_supported(self.single_pool(), "mark decommission failed")?; + self.decommission_failed_with_owner(idx, None).await + } - let (should_reload_pool_meta, previous_pool_meta) = { + async fn decommission_failed_for_operation(&self, idx: usize, owner: &DecommissionCanceler) -> Result<()> { + self.decommission_failed_with_owner(idx, Some(owner)).await + } + + async fn decommission_failed_with_owner(&self, idx: usize, owner: Option<&DecommissionCanceler>) -> Result<()> { + self.decommission_failed_with_owner_and_save(idx, owner, self.save_current_pool_meta()) + .await + } + + async fn decommission_failed_with_owner_and_save( + &self, + idx: usize, + owner: Option<&DecommissionCanceler>, + save_pool_meta: SaveFuture, + ) -> Result<()> + where + SaveFuture: Future>, + { + ensure_decommission_terminal_operation_supported(self.single_pool(), "mark decommission failed")?; + let _start_guard = self.start_gate.lock().await; + + // Lock order: decommission_cancelers before pool_meta. Holding both makes + // owner validation and the terminal transition one atomic operation. + let (should_reload_pool_meta, previous_pool_meta, terminal_canceler) = { + let cancelers = self.decommission_cancelers.read().await; let mut pool_meta = self.pool_meta.write().await; let previous_pool_meta = pool_meta.clone(); - let changed = pool_meta.decommission_failed(idx); - (changed, changed.then_some(previous_pool_meta)) + let Some(changed) = + update_decommission_for_operation(cancelers.as_slice(), &mut pool_meta, idx, owner, |pool_meta| { + pool_meta.decommission_failed(idx) + }) + else { + return Ok(()); + }; + let terminal_canceler = if let Some(owner) = owner { + Some(owner.clone()) + } else { + cancelers.get(idx).and_then(Option::as_ref).cloned() + }; + (changed, changed.then_some(previous_pool_meta), terminal_canceler) }; - { - let mut cancelers = self.decommission_cancelers.write().await; - take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), idx); - } - - if should_reload_pool_meta { - if let Err(err) = self.save_current_pool_meta().await { - if let Some(previous_pool_meta) = previous_pool_meta { - let mut pool_meta = self.pool_meta.write().await; - rollback_decommission_pool_meta(&mut pool_meta, previous_pool_meta); - } - return Err(err); + if should_reload_pool_meta && let Err(err) = save_pool_meta.await { + if let Some(previous_pool_meta) = previous_pool_meta { + let mut pool_meta = self.pool_meta.write().await; + rollback_decommission_pool_meta(&mut pool_meta, previous_pool_meta); } + return Err(err); + } + if should_reload_pool_meta { { let mut pool_meta = self.pool_meta.write().await; pool_meta.mark_decommission_progress_saved(); } - if let Some(notification_sys) = runtime_sources::notification_sys() { - let stage = format!("decommission_failed for pool {idx}"); - if let Some(err) = observe_decommission_terminal_reload_result( - resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str()), - stage.as_str(), - ) { - if let Err(record_err) = self - .record_decommission_terminal_reload_failure(idx, stage.as_str(), err.clone()) - .await - { - warn!( - event = EVENT_DECOMMISSION_STATE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_POOLS, - pool_index = idx, - state = "terminal_reload_record_failed", - error = %record_err, - original_error = %err, - "Decommission terminal reload failure record failed" - ); - } + } + if let Some(canceler) = terminal_canceler.as_ref() { + self.release_decommission_canceler_slot(idx, canceler).await; + } + if should_reload_pool_meta && let Some(notification_sys) = runtime_sources::notification_sys() { + let stage = format!("decommission_failed for pool {idx}"); + if let Some(err) = observe_decommission_terminal_reload_result( + resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str()), + stage.as_str(), + ) { + if let Err(record_err) = self + .record_decommission_terminal_reload_failure(idx, stage.as_str(), err.clone()) + .await + { warn!( event = EVENT_DECOMMISSION_STATE, component = LOG_COMPONENT_ECSTORE, subsystem = LOG_SUBSYSTEM_POOLS, pool_index = idx, - state = "terminal_reload_failed", - error = %err, - "Decommission terminal state saved but pool meta reload failed" + state = "terminal_reload_record_failed", + error = %record_err, + original_error = %err, + "Decommission terminal reload failure record failed" ); } + warn!( + event = EVENT_DECOMMISSION_STATE, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_POOLS, + pool_index = idx, + state = "terminal_reload_failed", + error = %err, + "Decommission terminal state saved but pool meta reload failed" + ); } } @@ -4019,63 +4330,84 @@ impl ECStore { #[tracing::instrument(skip(self))] pub async fn complete_decommission(&self, idx: usize) -> Result<()> { - ensure_decommission_terminal_operation_supported(self.single_pool(), "complete decommission")?; + self.complete_decommission_with_owner(idx, None).await + } - let (should_reload_pool_meta, previous_pool_meta) = { + async fn complete_decommission_for_operation(&self, idx: usize, owner: &DecommissionCanceler) -> Result<()> { + self.complete_decommission_with_owner(idx, Some(owner)).await + } + + async fn complete_decommission_with_owner(&self, idx: usize, owner: Option<&DecommissionCanceler>) -> Result<()> { + ensure_decommission_terminal_operation_supported(self.single_pool(), "complete decommission")?; + let _start_guard = self.start_gate.lock().await; + + // Lock order: decommission_cancelers before pool_meta. Holding both makes + // owner validation and the terminal transition one atomic operation. + let (should_reload_pool_meta, previous_pool_meta, terminal_canceler) = { + let cancelers = self.decommission_cancelers.read().await; let mut pool_meta = self.pool_meta.write().await; let previous_pool_meta = pool_meta.clone(); - let changed = pool_meta.decommission_complete(idx); - (changed, changed.then_some(previous_pool_meta)) + let Some(changed) = + update_decommission_for_operation(cancelers.as_slice(), &mut pool_meta, idx, owner, |pool_meta| { + pool_meta.decommission_complete(idx) + }) + else { + return Ok(()); + }; + let terminal_canceler = if let Some(owner) = owner { + Some(owner.clone()) + } else { + cancelers.get(idx).and_then(Option::as_ref).cloned() + }; + (changed, changed.then_some(previous_pool_meta), terminal_canceler) }; - { - let mut cancelers = self.decommission_cancelers.write().await; - take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), idx); - } - - if should_reload_pool_meta { - if let Err(err) = self.save_current_pool_meta().await { - if let Some(previous_pool_meta) = previous_pool_meta { - let mut pool_meta = self.pool_meta.write().await; - rollback_decommission_pool_meta(&mut pool_meta, previous_pool_meta); - } - return Err(err); + if should_reload_pool_meta && let Err(err) = self.save_current_pool_meta().await { + if let Some(previous_pool_meta) = previous_pool_meta { + let mut pool_meta = self.pool_meta.write().await; + rollback_decommission_pool_meta(&mut pool_meta, previous_pool_meta); } + return Err(err); + } + if should_reload_pool_meta { { let mut pool_meta = self.pool_meta.write().await; pool_meta.mark_decommission_progress_saved(); } - if let Some(notification_sys) = runtime_sources::notification_sys() { - let stage = format!("complete_decommission for pool {idx}"); - if let Some(err) = observe_decommission_terminal_reload_result( - resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str()), - stage.as_str(), - ) { - if let Err(record_err) = self - .record_decommission_terminal_reload_failure(idx, stage.as_str(), err.clone()) - .await - { - warn!( - event = EVENT_DECOMMISSION_STATE, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_POOLS, - pool_index = idx, - state = "terminal_reload_record_failed", - error = %record_err, - original_error = %err, - "Decommission terminal reload failure record failed" - ); - } + } + if let Some(canceler) = terminal_canceler.as_ref() { + self.release_decommission_canceler_slot(idx, canceler).await; + } + if should_reload_pool_meta && let Some(notification_sys) = runtime_sources::notification_sys() { + let stage = format!("complete_decommission for pool {idx}"); + if let Some(err) = observe_decommission_terminal_reload_result( + resolve_decommission_pool_meta_reload_result(notification_sys.reload_pool_meta().await, stage.as_str()), + stage.as_str(), + ) { + if let Err(record_err) = self + .record_decommission_terminal_reload_failure(idx, stage.as_str(), err.clone()) + .await + { warn!( event = EVENT_DECOMMISSION_STATE, component = LOG_COMPONENT_ECSTORE, subsystem = LOG_SUBSYSTEM_POOLS, pool_index = idx, - state = "terminal_reload_failed", - error = %err, - "Decommission terminal state saved but pool meta reload failed" + state = "terminal_reload_record_failed", + error = %record_err, + original_error = %err, + "Decommission terminal reload failure record failed" ); } + warn!( + event = EVENT_DECOMMISSION_STATE, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_POOLS, + pool_index = idx, + state = "terminal_reload_failed", + error = %err, + "Decommission terminal state saved but pool meta reload failed" + ); } } @@ -4189,6 +4521,23 @@ impl ECStore { #[tracing::instrument(skip(self))] pub async fn start_decommission(&self, indices: Vec) -> Result<()> { + self.start_decommission_inner(indices, None).await.map(|_| ()) + } + + async fn start_decommission_with_routines( + &self, + indices: Vec, + rx: &CancellationToken, + local_indices: &[usize], + ) -> Result> { + self.start_decommission_inner(indices, Some((rx, local_indices))).await + } + + async fn start_decommission_inner( + &self, + indices: Vec, + reservation: Option<(&CancellationToken, &[usize])>, + ) -> Result> { let indices = dedup_indices(&indices); validate_start_decommission_request(&indices, self.single_pool())?; @@ -4231,11 +4580,19 @@ impl ECStore { self.ensure_decommission_rebalance_idle_after_refresh().await?; let all_space_infos = self.get_decommission_all_pool_space_infos().await?; - { + let index_cancelers = if let Some((rx, local_indices)) = reservation { + // Lock order matches terminal transitions: decommission_cancelers + // before pool_meta while start_gate excludes another start. + let mut cancelers = self.decommission_cancelers.write().await; + let pool_meta = self.pool_meta.read().await; + ensure_decommission_start_target_capacity(&pool_meta, &indices, &all_space_infos)?; + reserve_decommission_start_cancelers(&pool_meta, &indices, local_indices, rx, cancelers.as_mut_slice())? + } else { let pool_meta = self.pool_meta.read().await; ensure_decommission_start_pool_states(&pool_meta, &indices)?; ensure_decommission_start_target_capacity(&pool_meta, &indices, &all_space_infos)?; - } + Vec::new() + }; let mut space_infos = Vec::with_capacity(indices.len()); for (idx, pi) in all_space_infos.iter().copied() { @@ -4311,7 +4668,7 @@ impl ECStore { return Err(Error::other(format!("{err}; decommission start rollback succeeded"))); } - Ok(()) + Ok(index_cancelers) } async fn get_buckets_to_decommission(&self) -> Result> { @@ -5457,10 +5814,13 @@ pub(crate) fn fallback_free_capacity_dedup(disks: &[rustfs_madmin::Disk]) -> usi #[cfg(test)] mod pools_tests { + use super::DECOMMISSION_PROGRESS_SAVE_RETRY_BACKOFF; + use super::record_decommission_entry_error; + use super::resolve_decommission_listing_error; use super::{ - DECOMMISSION_PROGRESS_SAVE_INTERVAL, DECOMMISSION_PROGRESS_SAVE_ITEM_THRESHOLD, DECOMMISSION_PROGRESS_SAVE_RETRY_BACKOFF, - DecomBucketInfo, DecommissionStartPoolState, DecommissionTerminalState, ListCallback, PoolDecommissionInfo, PoolMeta, - PoolSpaceInfo, PoolStatus, apply_decommission_status_space_info, bind_decommission_cancelers, + DECOMMISSION_PROGRESS_SAVE_INTERVAL, DECOMMISSION_PROGRESS_SAVE_ITEM_THRESHOLD, DecomBucketInfo, DecommissionCanceler, + DecommissionStartPoolState, DecommissionTerminalState, ListCallback, PoolDecommissionInfo, PoolMeta, PoolSpaceInfo, + PoolStatus, apply_decommission_status_space_info, await_decommission_worker, bind_decommission_cancelers, bind_missing_decommission_cancelers, cancel_decommission_canceler, classify_decommission_terminal_state, count_decommission_item, decommission_cancel_signal_result, decommission_item_size, decommission_meta_bucket_options, decommission_start_pool_state, dedup_indices, default_decommission_bucket_concurrency, @@ -5470,35 +5830,36 @@ mod pools_tests { ensure_decommission_start_rebalance_meta_allowed, ensure_decommission_start_target_capacity, ensure_decommission_terminal_operation_supported, ensure_local_decommission_pool_leaders, ensure_valid_decommission_pool_index, first_resumable_decommission_queue_indices, get_by_index, - has_active_decommission_canceler, is_decommission_active, is_decommission_cancel_requested, + guard_decommission_cancelers, has_active_decommission_canceler, is_decommission_active, is_decommission_cancel_requested, load_decommission_entry_versions, local_decommission_queue_prefix, mark_decommission_bucket_done, merge_pool_status_refresh, missing_decommission_worker_prefix, observe_decommission_terminal_reload_result, - pool_meta_has_active_decommission, record_decommission_entry_error, require_decommission_store, + pool_meta_has_active_decommission, require_decommission_store, reserve_decommission_start_cancelers, resolve_decommission_bucket_done_save_result, resolve_decommission_bucket_state, resolve_decommission_check_after_list_result, resolve_decommission_entry_cleanup_delete_result, - resolve_decommission_entry_exact_versions, resolve_decommission_entry_reload_result, resolve_decommission_listing_error, + resolve_decommission_entry_exact_versions, resolve_decommission_entry_reload_result, resolve_decommission_listing_worker_result, resolve_decommission_optional_bucket_config_result, resolve_decommission_partial_listing_entry, resolve_decommission_pool_meta_reload_result, resolve_decommission_preflight_heal_result, resolve_decommission_progress_save_result, - resolve_decommission_spawn_failure_result, resolve_decommission_terminal_mark_after_error_result, - resolve_decommission_terminal_mark_result, resolve_decommission_update_after_result, - resolve_start_decommission_pool_meta_reload_result, rollback_start_decommission_pool_meta, - run_decommission_buckets_bounded, run_decommission_listing_with_retry, should_cleanup_decommission_source_entry, - should_continue_decommission_queue, should_count_decommission_version_complete, + resolve_decommission_terminal_mark_after_error_result, resolve_decommission_terminal_mark_result, + resolve_decommission_update_after_result, resolve_start_decommission_pool_meta_reload_result, + rollback_start_decommission_pool_meta, run_decommission_buckets_bounded, run_decommission_listing_with_retry, + should_cleanup_decommission_source_entry, should_continue_decommission_queue, should_count_decommission_version_complete, should_preserve_decommission_canceled_state, should_reject_decommission_cancel_as_terminal, should_retry_decommission_cancel_reload, should_retry_decommission_listing, should_skip_canceled_decommission_routine, - split_decommission_buckets, take_and_cancel_decommission_canceler, take_decommission_canceler, - track_decommission_current_object, track_decommission_current_object_stage, validate_start_decommission_request, - wait_decommission_listing_retry, wait_decommission_worker_drain, with_decommission_entry_context, + spawn_decommission_index_cancelers, split_decommission_buckets, take_and_cancel_decommission_canceler, + take_decommission_canceler, track_decommission_current_object, track_decommission_current_object_stage, + update_decommission_for_operation, validate_start_decommission_request, wait_decommission_listing_retry, + wait_decommission_worker_drain, with_decommission_entry_context, }; use crate::data_movement; use crate::disk::endpoint::Endpoint; use crate::error::{Error, StorageError}; use crate::layout::endpoints::{EndpointServerPools, Endpoints, PoolEndpoints}; + use crate::runtime::instance::InstanceContext; use crate::services::rebalance::{RebalStatus, RebalanceInfo, RebalanceMeta, RebalanceStats}; - use rustfs_filemeta::{ - FileInfo, FileInfoVersions, MetaCacheEntries, MetaCacheEntry, MetadataResolutionParams, ObjectPartInfo, - }; + use crate::store::ECStore; + use rustfs_filemeta::{FileInfo, FileInfoVersions, MetaCacheEntry, ObjectPartInfo}; + use rustfs_filemeta::{MetaCacheEntries, MetadataResolutionParams}; use rustfs_rio::Index; use std::sync::{ Arc, @@ -5513,6 +5874,24 @@ mod pools_tests { Arc::new(|_| Box::pin(async {})) } + fn decommission_worker_test_store(pool_meta: PoolMeta, cancelers: Vec>) -> Arc { + let ctx = Arc::new(InstanceContext::new()); + let endpoint_pools = EndpointServerPools::default(); + Arc::new(ECStore { + id: uuid::Uuid::new_v4(), + disk_map: std::collections::HashMap::new(), + pools: Vec::new(), + peer_sys: crate::cluster::rpc::S3PeerSys::new_with_instance_ctx(&endpoint_pools, ctx.clone()), + pool_meta: tokio::sync::RwLock::new(pool_meta), + rebalance_meta: tokio::sync::RwLock::new(None), + decommission_cancelers: tokio::sync::RwLock::new(cancelers), + start_gate: tokio::sync::Mutex::new(()), + pool_meta_save_gate: tokio::sync::Mutex::new(()), + ctx, + bucket_fence_registry: Arc::default(), + }) + } + fn decommission_test_pool_endpoint(idx: usize, is_local: bool) -> PoolEndpoints { let port = 9000usize + idx; let mut endpoint = @@ -6383,20 +6762,6 @@ mod pools_tests { assert!(message.contains(Error::SlowDown.to_string().as_str())); } - #[test] - fn test_resolve_decommission_spawn_failure_result_keeps_primary_without_rollback_error() { - let err = resolve_decommission_spawn_failure_result(Error::SlowDown, None); - assert!(matches!(err, Error::SlowDown)); - } - - #[test] - fn test_resolve_decommission_spawn_failure_result_wraps_rollback_error() { - let err = resolve_decommission_spawn_failure_result(Error::SlowDown, Some(Error::OperationCanceled)); - let message = err.to_string(); - assert!(message.contains("decommission spawn routines failed")); - assert!(message.contains("rollback failed")); - } - #[test] fn test_decommission_item_size_converts_positive_values() { assert_eq!(decommission_item_size(42_i64), 42); @@ -8127,7 +8492,7 @@ mod pools_tests { #[test] fn test_bind_decommission_cancelers_replaces_existing_slot() { let parent = CancellationToken::new(); - let existing = CancellationToken::new(); + let existing = DecommissionCanceler::new(CancellationToken::new()); let mut cancelers = vec![Some(existing.clone())]; let bound = bind_decommission_cancelers(&[0], &parent, cancelers.as_mut_slice()); @@ -8144,7 +8509,7 @@ mod pools_tests { #[test] fn test_bind_missing_decommission_cancelers_stops_at_existing_slot() { let parent = CancellationToken::new(); - let existing = CancellationToken::new(); + let existing = DecommissionCanceler::new(CancellationToken::new()); let mut cancelers = vec![None, Some(existing.clone()), None]; let bound = bind_missing_decommission_cancelers(&[0, 1, 2], &parent, cancelers.as_mut_slice()); @@ -8157,6 +8522,38 @@ mod pools_tests { assert!(!existing.is_cancelled()); } + #[test] + fn test_serialized_decommission_double_start_preserves_first_operation() { + let mut pool_meta = PoolMeta { + pools: vec![decommission_test_pool_status(0, None), decommission_test_pool_status(1, None)], + ..Default::default() + }; + let first_parent = CancellationToken::new(); + let second_parent = CancellationToken::new(); + let mut cancelers = vec![None, None]; + + let first = reserve_decommission_start_cancelers(&pool_meta, &[0], &[0], &first_parent, cancelers.as_mut_slice()) + .expect("first start should reserve its worker"); + pool_meta + .decommission( + 0, + PoolSpaceInfo { + total: 100, + free: 40, + used: 60, + }, + ) + .expect("first start should install active metadata"); + + let second = reserve_decommission_start_cancelers(&pool_meta, &[0], &[0], &second_parent, cancelers.as_mut_slice()); + + assert!(matches!(second, Err(Error::DecommissionAlreadyRunning))); + let current = cancelers[0].as_ref().expect("first operation should retain the slot"); + assert!(current.owns_same_operation(first[0].1.canceler())); + assert!(current.is_active()); + assert!(!first_parent.is_cancelled()); + } + #[test] fn test_local_decommission_queue_prefix_stops_at_remote_leader() { let endpoints = EndpointServerPools::from(vec![ @@ -8207,7 +8604,7 @@ mod pools_tests { #[test] fn test_missing_decommission_worker_prefix_stops_at_active_worker() { - let cancelers = vec![None, Some(CancellationToken::new()), None]; + let cancelers = vec![None, Some(DecommissionCanceler::new(CancellationToken::new())), None]; let missing = missing_decommission_worker_prefix(&[0, 1, 2], cancelers.as_slice()); @@ -8316,8 +8713,8 @@ mod pools_tests { #[test] fn test_take_decommission_canceler_takes_and_clears_slot() { - let token = CancellationToken::new(); - let mut cancelers = vec![Some(token)]; + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let mut cancelers = vec![Some(canceler)]; let taken = take_decommission_canceler(cancelers.as_mut_slice(), 0); assert!(taken.is_some()); @@ -8326,13 +8723,13 @@ mod pools_tests { #[test] fn test_take_decommission_canceler_returns_none_for_missing_slot() { - let mut cancelers: Vec> = Vec::new(); + let mut cancelers: Vec> = Vec::new(); assert!(take_decommission_canceler(cancelers.as_mut_slice(), 0).is_none()); } #[test] fn test_has_active_decommission_canceler_true_when_any_slot_present() { - let cancelers = vec![None, Some(CancellationToken::new())]; + let cancelers = vec![None, Some(DecommissionCanceler::new(CancellationToken::new()))]; assert!(has_active_decommission_canceler(cancelers.as_slice())); } @@ -8344,11 +8741,12 @@ mod pools_tests { #[test] fn test_cancel_decommission_canceler_cancels_when_present() { - let token = CancellationToken::new(); - let canceled = cancel_decommission_canceler(Some(token.clone())); + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let canceled = cancel_decommission_canceler(Some(canceler.clone())); assert!(canceled); - assert!(token.is_cancelled()); + assert!(canceler.is_cancelled()); + assert!(!canceler.is_active()); } #[test] @@ -8358,12 +8756,13 @@ mod pools_tests { #[test] fn test_take_and_cancel_decommission_canceler_clears_slot() { - let token = CancellationToken::new(); - let mut cancelers = vec![Some(token.clone())]; + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let mut cancelers = vec![Some(canceler.clone())]; assert!(take_and_cancel_decommission_canceler(cancelers.as_mut_slice(), 0)); assert!(cancelers[0].is_none()); - assert!(token.is_cancelled()); + assert!(canceler.is_cancelled()); + assert!(!canceler.is_active()); } #[test] @@ -8374,6 +8773,195 @@ mod pools_tests { assert!(cancelers[0].is_none()); } + #[test] + fn test_guarded_decommission_future_releases_without_first_poll() { + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let cancelers = vec![Some(canceler.clone())]; + let guards = guard_decommission_cancelers(vec![(0, canceler.clone())]); + let unpolled = async move { + let _guards = guards; + std::future::pending::<()>().await; + }; + + drop(unpolled); + + assert!(canceler.is_cancelled()); + assert!(!has_active_decommission_canceler(cancelers.as_slice())); + } + + #[test] + fn test_partial_decommission_spawn_reservation_releases_bound_slot() { + let parent = CancellationToken::new(); + let mut cancelers = vec![None]; + let bound = bind_decommission_cancelers(&[0, 1], &parent, cancelers.as_mut_slice()); + let guards = guard_decommission_cancelers(bound); + + let result = super::ensure_decommission_routines_scheduled(guards.len(), 2); + drop(guards); + + assert!(result.is_err()); + assert!(!has_active_decommission_canceler(cancelers.as_slice())); + } + + #[tokio::test] + async fn test_decommission_supervisor_observes_worker_abort() { + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let worker = tokio::spawn(async move { + started_tx.send(()).expect("worker start should be observed"); + std::future::pending::<()>().await; + #[allow(unreachable_code)] + Ok(()) + }); + + started_rx.await.expect("worker start should be observed"); + worker.abort(); + let err = await_decommission_worker(3, worker) + .await + .expect_err("supervisor should observe aborted worker"); + + assert!(err.to_string().contains("decommission worker 3 task join error")); + } + + #[tokio::test] + async fn test_decommission_supervisor_observes_worker_panic() { + let worker = tokio::spawn(async move { + panic!("injected decommission worker panic"); + #[allow(unreachable_code)] + Ok(()) + }); + + let err = await_decommission_worker(4, worker) + .await + .expect_err("supervisor should observe panicked worker"); + + assert!(err.to_string().contains("decommission worker 4 task join error")); + } + + #[tokio::test] + async fn test_decommission_worker_metadata_missing_releases_owned_slot() { + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let store = decommission_worker_test_store(PoolMeta::default(), vec![Some(canceler.clone())]); + canceler.cancel(); + + let err = store + .do_decommission_in_routine(canceler.clone(), 0) + .await + .expect_err("missing worker metadata should fail the routine"); + + assert!(err.to_string().contains("target pool was not found")); + assert!(!canceler.is_active()); + assert!(store.decommission_cancelers.read().await[0].is_none()); + } + + #[tokio::test] + async fn test_decommission_supervisor_failure_cancels_queued_successor() { + let first = DecommissionCanceler::new(CancellationToken::new()); + let queued = DecommissionCanceler::new(CancellationToken::new()); + let store = decommission_worker_test_store(PoolMeta::default(), vec![Some(first.clone()), Some(queued.clone())]); + let guards = guard_decommission_cancelers(vec![(0, first.clone()), (1, queued.clone())]); + + spawn_decommission_index_cancelers(store.clone(), CancellationToken::new(), guards) + .await + .expect("decommission supervisor should finish after queued cleanup"); + + assert!(!first.is_active()); + assert!(!queued.is_active()); + assert!(queued.is_cancelled()); + assert!(store.decommission_cancelers.read().await.iter().all(Option::is_none)); + } + + #[tokio::test] + async fn test_decommission_failed_save_failure_preserves_owner_until_retry_succeeds() { + let canceler = DecommissionCanceler::new(CancellationToken::new()); + let pool_meta = PoolMeta { + pools: vec![decommission_test_pool_status( + 0, + Some(PoolDecommissionInfo { + start_time: Some(OffsetDateTime::UNIX_EPOCH), + ..Default::default() + }), + )], + ..Default::default() + }; + let store = decommission_worker_test_store(pool_meta, vec![Some(canceler.clone())]); + + store + .decommission_failed_with_owner_and_save(0, Some(&canceler), async { Err(Error::SlowDown) }) + .await + .expect_err("injected terminal save failure should be returned"); + + { + let cancelers = store.decommission_cancelers.read().await; + let current = cancelers[0].as_ref().expect("failed save must retain the exact owner slot"); + assert!(current.owns_same_operation(&canceler)); + assert!(current.is_active()); + } + { + let pool_meta = store.pool_meta.read().await; + let info = pool_meta.pools[0] + .decommission + .as_ref() + .expect("rollback must retain active decommission metadata"); + assert!(info.has_decommission_state()); + assert!(!info.failed); + assert!(!info.complete); + assert!(!info.canceled); + } + assert!(store.decommission_terminal_retryable_for_operation(0, &canceler).await); + + store + .decommission_failed_with_owner_and_save(0, Some(&canceler), async { Ok(()) }) + .await + .expect("terminal retry should commit"); + + let pool_meta = store.pool_meta.read().await; + assert!( + pool_meta.pools[0] + .decommission + .as_ref() + .expect("terminal metadata should remain") + .failed + ); + drop(pool_meta); + assert!(store.decommission_cancelers.read().await[0].is_none()); + assert!(!canceler.is_active()); + assert!(canceler.is_cancelled()); + } + + #[test] + fn test_stale_decommission_operation_cannot_cancel_replacement() { + let stale = DecommissionCanceler::new(CancellationToken::new()); + let replacement = DecommissionCanceler::new(CancellationToken::new()); + let cancelers = vec![Some(replacement.clone())]; + let mut pool_meta = PoolMeta { + pools: vec![decommission_test_pool_status( + 0, + Some(PoolDecommissionInfo { + start_time: Some(OffsetDateTime::UNIX_EPOCH), + ..Default::default() + }), + )], + ..Default::default() + }; + + let changed = update_decommission_for_operation(cancelers.as_slice(), &mut pool_meta, 0, Some(&stale), |pool_meta| { + pool_meta.decommission_cancel(0) + }); + + assert!(changed.is_none()); + assert!( + !pool_meta.pools[0] + .decommission + .as_ref() + .expect("replacement metadata should remain") + .canceled + ); + assert!(replacement.is_active()); + assert!(!replacement.is_cancelled()); + assert!(!stale.is_active()); + assert!(stale.is_cancelled()); + } + #[test] fn test_ensure_decommission_routines_scheduled_accepts_positive_bound_count() { assert!(super::ensure_decommission_routines_scheduled(2, 2).is_ok()); diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index 4b8a9cb71..757be0c53 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -33,7 +33,7 @@ use crate::bucket::utils::check_put_object_part_args; use crate::bucket::utils::{check_valid_bucket_name, check_valid_bucket_name_strict, is_meta_bucketname}; use crate::cluster::rpc::{RemoteClient, S3PeerSys}; use crate::config::storageclass; -use crate::core::pools::PoolMeta; +use crate::core::pools::{DecommissionCanceler, PoolMeta}; use crate::disk::endpoint::{Endpoint, EndpointType}; use crate::disk::{DiskAPI, DiskInfo, DiskInfoOptions}; use crate::error::{Error, Result}; @@ -176,7 +176,7 @@ pub struct ECStore { // pub local_disks: Vec, pub pool_meta: RwLock, pub rebalance_meta: RwLock>, - pub decommission_cancelers: RwLock>>, + pub decommission_cancelers: RwLock>>, /// Serializes rebalance/decommission start transitions. /// /// Lock order: acquire `start_gate` before `pool_meta`, `rebalance_meta`,