From 0f987714a19afe5264ee3377a72d9119f3fa58f9 Mon Sep 17 00:00:00 2001 From: houseme Date: Wed, 26 Aug 2026 09:17:19 +0800 Subject: [PATCH] fix(ecstore): handle metadata-less bucket residue (#6591) * fix(ecstore): handle metadata-less bucket residue Diagnose metadata-less on-disk residue before non-force DeleteBucket reaches physical deletion, and keep scanner-discovered metadata-missing objects on a non-destructive heal path. Add explicit heal --remove cleanup for unversioned metadata-less data directories, using the existing data-dir delete primitive and fail-closed shape checks so pre-commit or unknown residue is preserved. Co-Authored-By: heihutu * fix(connect): adapt offline array validator Wrap the filesystem summary validator in a closure so Option::is_some_and can pass the concrete array reference accepted by serde_json::Value::as_array. Co-Authored-By: heihutu * fix(connect): remove redundant offline test clones Move the temporary path into the swap closure after deriving the output path, keeping clippy's redundant-clone lint clean for offline bundle tests. Co-Authored-By: heihutu --------- Co-authored-by: heihutu --- crates/ecstore/src/error/mod.rs | 8 +- crates/ecstore/src/set_disk/ops/heal.rs | 377 +++++++++++++++++++- crates/ecstore/src/store/bucket.rs | 145 +++++++- crates/ecstore/src/store/mod.rs | 69 ++++ crates/scanner/src/scanner_folder.rs | 2 +- rustfs/src/connect/offline/bundle_writer.rs | 6 +- rustfs/src/error.rs | 9 +- 7 files changed, 595 insertions(+), 21 deletions(-) diff --git a/crates/ecstore/src/error/mod.rs b/crates/ecstore/src/error/mod.rs index f8215d908..c12d166c0 100644 --- a/crates/ecstore/src/error/mod.rs +++ b/crates/ecstore/src/error/mod.rs @@ -119,6 +119,8 @@ pub enum StorageError { BucketExists(String), #[error("Bucket not empty: {0}")] BucketNotEmpty(String), + #[error("Bucket not empty: {bucket} ({details})")] + BucketNotEmptyWithDetails { bucket: String, details: String }, #[error("Bucket name invalid: {0}")] BucketNameInvalid(String), @@ -493,6 +495,10 @@ impl Clone for StorageError { StorageError::MethodNotAllowed => StorageError::MethodNotAllowed, StorageError::BucketNotFound(a) => StorageError::BucketNotFound(a.clone()), StorageError::BucketNotEmpty(a) => StorageError::BucketNotEmpty(a.clone()), + StorageError::BucketNotEmptyWithDetails { bucket, details } => StorageError::BucketNotEmptyWithDetails { + bucket: bucket.clone(), + details: details.clone(), + }, StorageError::BucketNameInvalid(a) => StorageError::BucketNameInvalid(a.clone()), StorageError::ObjectNameInvalid(a, b) => StorageError::ObjectNameInvalid(a.clone(), b.clone()), StorageError::BucketExists(a) => StorageError::BucketExists(a.clone()), @@ -600,7 +606,7 @@ impl StorageError { StorageError::InvalidArgument(_, _, _) => StorageErrorCode::InvalidArgument, StorageError::MethodNotAllowed => StorageErrorCode::MethodNotAllowed, StorageError::BucketNotFound(_) => StorageErrorCode::BucketNotFound, - StorageError::BucketNotEmpty(_) => StorageErrorCode::BucketNotEmpty, + StorageError::BucketNotEmpty(_) | StorageError::BucketNotEmptyWithDetails { .. } => StorageErrorCode::BucketNotEmpty, StorageError::BucketNameInvalid(_) => StorageErrorCode::BucketNameInvalid, StorageError::ObjectNameInvalid(_, _) => StorageErrorCode::ObjectNameInvalid, StorageError::BucketExists(_) => StorageErrorCode::BucketExists, diff --git a/crates/ecstore/src/set_disk/ops/heal.rs b/crates/ecstore/src/set_disk/ops/heal.rs index 67be1ce57..65fcfdbd6 100644 --- a/crates/ecstore/src/set_disk/ops/heal.rs +++ b/crates/ecstore/src/set_disk/ops/heal.rs @@ -13,7 +13,9 @@ // limitations under the License. use super::super::*; +use crate::disk::DataDirDeleteStatus; use crate::disk::disk_store::DiskStoreRenameDataExt; +use crate::disk::local::DELETE_DATA_DIR_MARKER_PREFIX; use crate::io_support::bitrot::object_mmap_read_enabled; use crate::storage_api_contracts::namespace::NamespaceLocking as _; use rustfs_common::trace_bus::{TraceEvent, TraceFunc, TraceKind, trace_emit}; @@ -172,6 +174,21 @@ enum DanglingDeleteSafety { NoRecoverableCandidate, } +#[derive(Debug, Default)] +struct MetadataLessDataDirCleanup { + matched: usize, + removed: usize, + first_error: Option, + delete_errs: Vec>, + touched_disks: Vec, +} + +fn metadata_less_part_file(entry: &str) -> bool { + entry + .strip_prefix("part.") + .is_some_and(|part_number| part_number.parse::().is_ok_and(|part_number| part_number > 0)) +} + #[cfg(test)] struct DanglingCheckPartsFailure { key: DanglingCheckPartsFailureKey, @@ -504,6 +521,21 @@ impl SetDisks { } else { DiskError::FileNotFound }; + if version_id.is_empty() + && (opts.remove || opts.dry_run) + && let Some(cleanup) = self + .cleanup_metadata_less_data_dirs(bucket, object, &disks, opts.dry_run) + .await? + { + let mut result = self + .metadata_less_data_dir_heal_result(bucket, object, &cleanup, opts.dry_run) + .await; + result.detail = format!( + "metadata-less data directories matched={}, removed={}, dry_run={}", + cleanup.matched, cleanup.removed, opts.dry_run + ); + return Ok((result, cleanup.first_error.or(Some(err)))); + } // Nothing to do, file is already gone. return Ok(( self.default_heal_result(FileInfo::default(), &errs, bucket, object, version_id) @@ -1480,6 +1512,183 @@ impl SetDisks { } } + async fn cleanup_metadata_less_data_dirs( + &self, + bucket: &str, + object: &str, + disks: &[Option], + dry_run: bool, + ) -> disk::error::Result> { + if disks.iter().any(Option::is_none) { + return Ok(None); + } + + let mut per_disk_dirs = Vec::new(); + for (disk_index, disk) in disks.iter().enumerate() { + let Some(disk) = disk.as_ref() else { + return Ok(None); + }; + let entries = match disk.list_dir("", bucket, object, -1).await { + Ok(entries) => entries, + Err( + DiskError::FileNotFound + | DiskError::FileVersionNotFound + | DiskError::PathNotFound + | DiskError::VolumeNotFound, + ) => continue, + Err(err) => return Err(err), + }; + for entry in entries { + let data_dir = entry.trim_end_matches(SLASH_SEPARATOR); + if !entry.ends_with(SLASH_SEPARATOR) || Uuid::parse_str(data_dir).is_err() { + return Ok(None); + } + let data_dir_path = format!("{object}{SLASH_SEPARATOR}{data_dir}"); + if !Self::metadata_less_data_dir_is_reclaimable(disk, bucket, &data_dir_path).await? { + return Ok(None); + } + per_disk_dirs.push((disk_index, data_dir_path)); + } + } + + let matched = per_disk_dirs.len(); + if matched == 0 { + return Ok(None); + } + + let mut cleanup = MetadataLessDataDirCleanup { + matched, + delete_errs: vec![None; disks.len()], + touched_disks: vec![false; disks.len()], + ..Default::default() + }; + for (disk_index, _) in &per_disk_dirs { + cleanup.touched_disks[*disk_index] = true; + } + + if dry_run { + return Ok(Some(cleanup)); + } + + for (disk_index, data_dir_path) in per_disk_dirs { + let Some(disk) = disks[disk_index].as_ref() else { + return Ok(None); + }; + match disk + .delete_data_dir( + bucket, + &data_dir_path, + DeleteOptions { + recursive: true, + ..Default::default() + }, + ) + .await + { + Ok(DataDirDeleteStatus::Deleted) => { + cleanup.removed = cleanup.removed.saturating_add(1); + } + Ok(DataDirDeleteStatus::Deferred) => { + let err = DiskError::FileAccessDenied; + cleanup.first_error.get_or_insert_with(|| err.clone()); + cleanup.delete_errs[disk_index] = Some(err); + } + Err(DiskError::FileNotFound | DiskError::PathNotFound | DiskError::VolumeNotFound) => { + cleanup.removed = cleanup.removed.saturating_add(1); + } + Err(err) => { + cleanup.first_error.get_or_insert_with(|| err.clone()); + cleanup.delete_errs[disk_index] = Some(err); + } + } + } + + Ok(Some(cleanup)) + } + + async fn metadata_less_data_dir_is_reclaimable( + disk: &DiskStore, + bucket: &str, + data_dir_path: &str, + ) -> disk::error::Result { + let entries = match disk.list_dir("", bucket, data_dir_path, -1).await { + Ok(entries) => entries, + Err( + DiskError::FileNotFound | DiskError::FileVersionNotFound | DiskError::PathNotFound | DiskError::VolumeNotFound, + ) => return Ok(false), + Err(err) => return Err(err), + }; + + let mut has_part = false; + for entry in entries { + if entry.ends_with(SLASH_SEPARATOR) { + return Ok(false); + } + let entry_name = entry.trim_end_matches(SLASH_SEPARATOR); + if metadata_less_part_file(entry_name) { + has_part = true; + continue; + } + if entry_name + .strip_prefix(DELETE_DATA_DIR_MARKER_PREFIX) + .is_some_and(|transaction| Uuid::parse_str(transaction).is_ok()) + { + continue; + } + return Ok(false); + } + + Ok(has_part) + } + + async fn metadata_less_data_dir_heal_result( + &self, + bucket: &str, + object: &str, + cleanup: &MetadataLessDataDirCleanup, + dry_run: bool, + ) -> HealResultItem { + let mut result = HealResultItem { + heal_item_type: HealItemType::Object.to_string(), + bucket: bucket.to_string(), + object: object.to_string(), + disk_count: self.set_endpoints.len(), + parity_blocks: self.default_parity_count, + data_blocks: self.set_endpoints.len().saturating_sub(self.default_parity_count), + ..Default::default() + }; + result.before.drives = Vec::with_capacity(self.set_endpoints.len()); + result.after.drives = Vec::with_capacity(self.set_endpoints.len()); + for (index, drive) in self.set_endpoints.iter().enumerate() { + let endpoint = drive.to_string(); + let before_state = if cleanup.touched_disks.get(index).copied().unwrap_or(false) { + DriveState::Corrupt.to_string() + } else { + DriveState::Missing.to_string() + }; + let after_state = if dry_run { + before_state.clone() + } else { + match cleanup.delete_errs.get(index).and_then(Option::as_ref) { + None => DriveState::Missing.to_string(), + Some(DiskError::DiskNotFound) => DriveState::Offline.to_string(), + Some(_) => DriveState::Corrupt.to_string(), + } + }; + result.before.drives.push(HealDriveInfo { + uuid: String::new(), + endpoint: endpoint.clone(), + state: before_state, + }); + result.after.drives.push(HealDriveInfo { + uuid: String::new(), + endpoint, + state: after_state, + }); + } + result + } + /// Prevent dangling cleanup when surviving state cannot prove that deletion /// is safe. Part presence proves only recoverability, never commit: the write /// path can durably rename data before xl.meta is committed. @@ -2006,6 +2215,23 @@ impl crate::storage_api_contracts::heal::HealOperations for SetDisks { } else { Error::FileNotFound }; + if version_id.is_empty() + && (opts.remove || opts.dry_run) + && let Some(cleanup) = self + .cleanup_metadata_less_data_dirs(bucket, object, &disks, opts.dry_run) + .await + .map_err(|e| to_object_err(e.into(), vec![bucket, object]))? + { + let mut result = self + .metadata_less_data_dir_heal_result(bucket, object, &cleanup, opts.dry_run) + .await; + result.detail = format!( + "metadata-less data directories matched={}, removed={}, dry_run={}", + cleanup.matched, cleanup.removed, opts.dry_run + ); + let err = cleanup.first_error.map(Error::from).or(Some(err)); + return Ok((result, err)); + } return Ok(( self.default_heal_result(FileInfo::default(), &errs, bucket, object, version_id) .await, @@ -2118,11 +2344,12 @@ mod heal_result_report_tests { use crate::disk::endpoint::Endpoint; use crate::disk::error::DiskError; use crate::disk::format::FormatV3; + use crate::disk::local::RESERVED_DELETE_DATA_DIR_MARKER_PREFIX; use crate::disk::{DiskAPI as _, DiskOption, DiskStore, RUSTFS_META_TMP_BUCKET, ReadOptions, STORAGE_FORMAT_FILE, new_disk}; use crate::error::Error; use crate::object_api::{ObjectOptions, PutObjReader}; use crate::set_disk::ops::object::hermetic_set_disks_support::hermetic_set_disks_isolated; - use crate::storage_api_contracts::bucket::{BucketOperations as _, MakeBucketOptions}; + use crate::storage_api_contracts::bucket::{BucketOperations as _, DeleteBucketOptions, MakeBucketOptions}; use crate::storage_api_contracts::heal::HealOperations as _; use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _}; use crate::{ @@ -2139,6 +2366,16 @@ mod heal_result_report_tests { use tracing_subscriber::fmt::MakeWriter; use uuid::Uuid; + #[test] + fn metadata_less_part_file_accepts_positive_part_numbers_only() { + assert!(super::metadata_less_part_file("part.1")); + assert!(super::metadata_less_part_file("part.42")); + assert!(!super::metadata_less_part_file("part.0")); + assert!(!super::metadata_less_part_file("part.")); + assert!(!super::metadata_less_part_file("part.x")); + assert!(!super::metadata_less_part_file("xl.meta")); + } + #[derive(Clone, Default)] struct CapturedLogs { buffer: Arc>>, @@ -2944,6 +3181,144 @@ mod heal_result_report_tests { ); } + async fn write_metadata_less_data_dir(root: &std::path::Path, bucket: &str, object: &str, data_dir: Uuid) { + let data_dir_path = root.join(bucket).join(object).join(data_dir.to_string()); + tokio::fs::create_dir_all(&data_dir_path) + .await + .expect("metadata-less data dir should be created"); + tokio::fs::write(data_dir_path.join("part.1"), b"orphaned-shard") + .await + .expect("metadata-less part should be written"); + } + + #[tokio::test] + async fn heal_object_remove_reclaims_metadata_less_data_dirs() { + let bucket = "bucket-metadata-less-remove"; + let object = "object.bin"; + let mut temp_dirs = Vec::new(); + let mut endpoints = Vec::new(); + let mut disks = Vec::new(); + let mut data_dirs = Vec::new(); + for _ in 0..2 { + let (temp_dir, endpoint, disk) = real_disk().await; + disk.make_volume(bucket).await.expect("test bucket should be created"); + let data_dir = Uuid::new_v4(); + write_metadata_less_data_dir(temp_dir.path(), bucket, object, data_dir).await; + data_dirs.push(data_dir); + temp_dirs.push(temp_dir); + endpoints.push(endpoint); + disks.push(Some(disk)); + } + let set = set_disks_with(disks, endpoints, 1).await; + + let (result, err) = set + .heal_object( + bucket, + object, + "", + &HealOpts { + remove: true, + no_lock: true, + ..Default::default() + }, + ) + .await + .expect("metadata-less remove should complete"); + + assert_eq!(err, Some(DiskError::FileNotFound)); + assert!( + result.detail.contains("matched=2, removed=2"), + "heal result should report reclaimed metadata-less dirs: {}", + result.detail + ); + for (temp_dir, data_dir) in temp_dirs.iter().zip(data_dirs.iter()) { + assert!( + !temp_dir.path().join(bucket).join(object).join(data_dir.to_string()).exists(), + "metadata-less data dir should be removed" + ); + } + + set.delete_bucket(bucket, &DeleteBucketOptions::default()) + .await + .expect("metadata-less data-dir cleanup should unblock non-force bucket deletion"); + } + + #[tokio::test] + async fn heal_object_remove_dry_run_reports_metadata_less_data_dirs_without_deleting() { + let bucket = "bucket-metadata-less-dry-run"; + let object = "object.bin"; + let (temp_dir, endpoint, disk) = real_disk().await; + disk.make_volume(bucket).await.expect("test bucket should be created"); + let data_dir = Uuid::new_v4(); + write_metadata_less_data_dir(temp_dir.path(), bucket, object, data_dir).await; + let set = set_disks_with(vec![Some(disk)], vec![endpoint], 0).await; + + let (result, err) = set + .heal_object( + bucket, + object, + "", + &HealOpts { + dry_run: true, + remove: true, + no_lock: true, + ..Default::default() + }, + ) + .await + .expect("metadata-less dry-run should complete"); + + assert_eq!(err, Some(DiskError::FileNotFound)); + assert!( + result.detail.contains("matched=1, removed=0, dry_run=true"), + "dry-run should report a match without deletion: {}", + result.detail + ); + assert!( + temp_dir.path().join(bucket).join(object).join(data_dir.to_string()).exists(), + "dry-run must preserve metadata-less data dir" + ); + } + + #[tokio::test] + async fn heal_object_remove_preserves_metadata_less_data_dir_with_precommit_marker() { + let bucket = "bucket-metadata-less-precommit"; + let object = "object.bin"; + let (temp_dir, endpoint, disk) = real_disk().await; + disk.make_volume(bucket).await.expect("test bucket should be created"); + let data_dir = Uuid::new_v4(); + write_metadata_less_data_dir(temp_dir.path(), bucket, object, data_dir).await; + let data_dir_path = temp_dir.path().join(bucket).join(object).join(data_dir.to_string()); + tokio::fs::write( + data_dir_path.join(format!("{}{}", RESERVED_DELETE_DATA_DIR_MARKER_PREFIX, Uuid::new_v4())), + [], + ) + .await + .expect("pre-commit reservation marker should be written"); + let set = set_disks_with(vec![Some(disk)], vec![endpoint], 0).await; + + let (result, err) = set + .heal_object( + bucket, + object, + "", + &HealOpts { + remove: true, + no_lock: true, + ..Default::default() + }, + ) + .await + .expect("metadata-less precommit state should fail closed"); + + assert_eq!(err, Some(DiskError::FileNotFound)); + assert!( + result.detail.is_empty(), + "pre-commit residue must not be classified as reclaimable metadata-less data" + ); + assert!(data_dir_path.exists(), "pre-commit residue must be preserved"); + } + #[tokio::test] async fn dangling_delete_guard_preserves_conflicting_identities_without_writing_metadata() { let bucket = "bucket-delete-guard-conflict"; diff --git a/crates/ecstore/src/store/bucket.rs b/crates/ecstore/src/store/bucket.rs index 1631ba0c7..fcda7ffbc 100644 --- a/crates/ecstore/src/store/bucket.rs +++ b/crates/ecstore/src/store/bucket.rs @@ -153,6 +153,38 @@ where await_bucket_namespace_operation(guard, bucket, "physical bucket deletion", future).await } +async fn bucket_delete_local_blocker( + ctx: &crate::runtime::instance::InstanceContext, + bucket: &str, +) -> Result> { + let local_disks = runtime_sources::local_disks_in(ctx).await; + let mut residue = BucketMetadataLessResidue::default(); + + for disk in local_disks.iter() { + let Some(bucket_path) = disk.get_bucket_path_for_io_if_local(bucket) else { + continue; + }; + let scan = scan_metadata_less_residue(&bucket_path?).await?; + if scan.xlmeta_found { + return Ok(Some(StorageError::BucketNotEmpty(bucket.to_string()))); + } + residue.files = residue.files.saturating_add(scan.files); + residue.uuid_data_dirs = residue.uuid_data_dirs.saturating_add(scan.uuid_data_dirs); + if residue.sample.is_none() { + residue.sample = scan.sample; + } + } + + if residue.has_residue_without_xlmeta() { + return Ok(Some(StorageError::BucketNotEmptyWithDetails { + bucket: bucket.to_string(), + details: residue.describe(), + })); + } + + Ok(None) +} + impl ECStore { pub async fn get_bucket_metadata(&self, bucket: &str) -> Result> { let sys = metadata_sys::require_bucket_metadata_sys_in(&self.ctx)?; @@ -755,20 +787,9 @@ impl ECStore { if bucket_exists { validate_table_bucket_delete_guard(&self.ctx, bucket).await?; - // Check bucket is empty before deletion (per S3 API spec) - // If bucket is not empty (contains actual objects with xl.meta files) and force - // is not set, return BucketNotEmpty error. - // Note: Empty directories (left after object deletion) should NOT count as objects. if !opts.force { - let local_disks = runtime_sources::local_disks_in(&self.ctx).await; - for disk in local_disks.iter() { - let Some(bucket_path) = disk.get_bucket_path_for_io_if_local(bucket) else { - continue; - }; - let bucket_path = bucket_path?; - if has_xlmeta_files(&bucket_path).await? { - return Err(StorageError::BucketNotEmpty(bucket.to_string())); - } + if let Some(blocker) = bucket_delete_local_blocker(&self.ctx, bucket).await? { + return Err(blocker); } delete_opts.force_if_empty = true; } @@ -804,6 +825,12 @@ impl ECStore { if let Err(err) = delete_result && (!sr_delete || !is_err_strict_volume_not_found(&err)) { + if delete_opts.force_if_empty + && matches!(&err, StorageError::BucketNotEmpty(_)) + && let Some(blocker) = bucket_delete_local_blocker(&self.ctx, bucket).await? + { + return Err(blocker); + } return Err(err); } @@ -831,7 +858,8 @@ mod tests { use super::{ SCANNER_BUCKET_LIST_SET_CONCURRENCY, await_bucket_namespace_operation, bucket_delete_metadata_cleanup_prefixes, bucket_deleted_marker_prefix, bucket_deleted_marker_volume, run_bucket_usage_cleanup, run_physical_bucket_deletion, - scanner_bucket_list_set_concurrency, should_override_created_from_metadata, validate_table_bucket_delete_allowed, + scan_metadata_less_residue, scanner_bucket_list_set_concurrency, should_override_created_from_metadata, + validate_table_bucket_delete_allowed, }; use crate::bucket::metadata::table_bucket_catalog_metadata_prefix; use crate::bucket::metadata_sys; @@ -1183,6 +1211,19 @@ mod tests { false } + async fn write_metadata_less_part_on_all_disks(disk_paths: &[PathBuf], bucket: &str, object: &str) { + for disk_path in disk_paths { + let data_dir = Uuid::new_v4(); + let part_path = disk_path.join(bucket).join(object).join(data_dir.to_string()).join("part.1"); + tokio::fs::create_dir_all(part_path.parent().expect("part path should have a parent")) + .await + .expect("metadata-less data dir should be created"); + tokio::fs::write(part_path, b"orphan shard") + .await + .expect("metadata-less part should be written"); + } + } + async fn write_bucket_metadata_marker(disk_paths: &[PathBuf], metadata_prefix: &str) { for disk_path in disk_paths { let marker_path = disk_path.join(metadata_prefix).join("config.json"); @@ -1239,6 +1280,39 @@ mod tests { assert_eq!(scanner_bucket_list_set_concurrency(100), SCANNER_BUCKET_LIST_SET_CONCURRENCY); } + #[tokio::test] + async fn metadata_less_residue_scan_ignores_empty_dirs_and_reports_uuid_data() { + let root = tempfile::tempdir().expect("temporary bucket root should be created"); + let bucket_path = root.path().join("bucket"); + tokio::fs::create_dir_all(bucket_path.join("empty/child")) + .await + .expect("empty directory residue should be created"); + + let empty = scan_metadata_less_residue(&bucket_path) + .await + .expect("empty residue scan should succeed"); + assert!(!empty.has_residue_without_xlmeta()); + + let data_dir = Uuid::new_v4(); + let part_path = bucket_path.join("object").join(data_dir.to_string()).join("part.1"); + tokio::fs::create_dir_all(part_path.parent().expect("part path should have a parent")) + .await + .expect("data dir should be created"); + tokio::fs::write(&part_path, b"orphan shard") + .await + .expect("part file should be written"); + + let residue = scan_metadata_less_residue(&bucket_path) + .await + .expect("metadata-less residue scan should succeed"); + assert!(residue.has_residue_without_xlmeta()); + assert_eq!(residue.files, 1); + assert_eq!(residue.uuid_data_dirs, 1); + let sample = residue.sample.as_deref().expect("part sample should be recorded"); + assert!(sample.starts_with("object/")); + assert!(sample.ends_with("/part.1")); + } + #[tokio::test] #[serial] async fn scanner_bucket_listing_unions_every_erasure_set() { @@ -1760,6 +1834,49 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn bucket_delete_reports_metadata_less_residue_without_removing_it() { + let (disk_paths, ecstore) = setup_bucket_delete_test_env().await; + let bucket = format!("bucket-delete-orphan-datadir-{}", Uuid::new_v4().simple()); + let object = "object.txt"; + + ecstore + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + write_metadata_less_part_on_all_disks(&disk_paths, &bucket, object).await; + assert!( + !any_disk_has_object_metadata(&disk_paths, &bucket).await, + "test setup must reproduce a bucket with data dirs but no xl.meta" + ); + + let err = ecstore + .delete_bucket(&bucket, &DeleteBucketOptions::default()) + .await + .expect_err("metadata-less data dirs must block ordinary DeleteBucket"); + + match err { + StorageError::BucketNotEmptyWithDetails { + bucket: err_bucket, + details, + } => { + assert_eq!(err_bucket, bucket); + assert!( + details.contains("metadata-less on-disk residue"), + "diagnostic should identify metadata-less residue, got: {details}" + ); + assert!(details.contains("files="), "diagnostic should include a bounded count"); + assert!(details.contains("uuid_data_dirs="), "diagnostic should include data-dir count"); + } + other => panic!("expected detailed BucketNotEmpty, got {other:?}"), + } + assert!( + any_disk_path_exists(&disk_paths, Path::new(&bucket).join(object)).await, + "ordinary DeleteBucket must preserve metadata-less data until an explicit operator action" + ); + } + #[tokio::test] #[serial] async fn bucket_delete_fences_put_started_after_empty_scan() { diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index a3d5df0d0..6e8ba2e9b 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -89,6 +89,28 @@ type WalkOptions = StorageWalkOptions bool>; pub const SCANNER_PUBLICATION_LEASE_TTL_MS: u64 = 60_000; +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub(crate) struct BucketMetadataLessResidue { + pub(crate) xlmeta_found: bool, + pub(crate) files: usize, + pub(crate) uuid_data_dirs: usize, + pub(crate) sample: Option, +} + +impl BucketMetadataLessResidue { + pub(crate) fn has_residue_without_xlmeta(&self) -> bool { + !self.xlmeta_found && self.files > 0 + } + + pub(crate) fn describe(&self) -> String { + let sample = self.sample.as_deref().unwrap_or(""); + format!( + "metadata-less on-disk residue remains after empty-bucket verification: files={}, uuid_data_dirs={}, sample={sample}", + self.files, self.uuid_data_dirs + ) + } +} + /// Check if a directory contains any xl.meta files (indicating actual S3 objects) /// This is used to determine if a bucket is empty for deletion purposes. pub(crate) async fn has_xlmeta_files(path: &std::path::Path) -> std::io::Result { @@ -123,6 +145,53 @@ pub(crate) async fn has_xlmeta_files(path: &std::path::Path) -> std::io::Result< Ok(false) } +pub(crate) async fn scan_metadata_less_residue(path: &std::path::Path) -> std::io::Result { + use crate::disk::STORAGE_FORMAT_FILE; + use tokio::fs; + + let mut scan = BucketMetadataLessResidue::default(); + let mut stack = vec![path.to_path_buf()]; + + while let Some(current_path) = stack.pop() { + let mut entries = match fs::read_dir(¤t_path).await { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => return Err(err), + }; + + while let Some(entry) = entries.next_entry().await? { + let file_type = entry.file_type().await?; + let file_name = entry.file_name(); + let file_name_str = file_name.to_string_lossy(); + + if file_name_str == STORAGE_FORMAT_FILE { + scan.xlmeta_found = true; + continue; + } + + if file_type.is_dir() { + if Uuid::parse_str(&file_name_str).is_ok() { + scan.uuid_data_dirs = scan.uuid_data_dirs.saturating_add(1); + } + stack.push(entry.path()); + } else { + scan.files = scan.files.saturating_add(1); + if scan.sample.is_none() { + let entry_path = entry.path(); + let sample = entry_path + .strip_prefix(path) + .unwrap_or(entry_path.as_path()) + .to_string_lossy() + .replace(std::path::MAIN_SEPARATOR, "/"); + scan.sample = Some(sample); + } + } + } + } + + Ok(scan) +} + async fn enqueue_transition_after_write(result: Result, src: LcEventSrc) -> Result { match result { Ok(oi) => { diff --git a/crates/scanner/src/scanner_folder.rs b/crates/scanner/src/scanner_folder.rs index c709fc436..efc0ca26e 100644 --- a/crates/scanner/src/scanner_folder.rs +++ b/crates/scanner/src/scanner_folder.rs @@ -1648,7 +1648,7 @@ impl FolderScanner { bucket.clone(), Some(object.clone()), None, - build_object_heal_request(bucket, object, None, self.scan_mode, HealChannelPriority::High), + build_non_destructive_object_heal_request(bucket, object, self.scan_mode, HealChannelPriority::High), ) .await?; } diff --git a/rustfs/src/connect/offline/bundle_writer.rs b/rustfs/src/connect/offline/bundle_writer.rs index 877c1bfc7..9456a03d5 100644 --- a/rustfs/src/connect/offline/bundle_writer.rs +++ b/rustfs/src/connect/offline/bundle_writer.rs @@ -335,7 +335,7 @@ fn valid_payload(collector: OfflineCollector, value: &serde_json::Value) -> bool && object.get("totalBytes").and_then(serde_json::Value::as_u64).is_some() && object.get("underPressure").and_then(serde_json::Value::as_bool).is_some() }), - OfflineCollector::FilesystemSummary => value.as_array().is_some_and(ordered_strings), + OfflineCollector::FilesystemSummary => value.as_array().is_some_and(|values| ordered_strings(values)), OfflineCollector::NetworkSummary => value.as_object().is_some_and(|object| { object.len() == 2 && object.get("bondCount").and_then(serde_json::Value::as_u64).is_some() @@ -902,7 +902,7 @@ mod tests { let moved = temp.path().join("moved"); fs::create_dir(&original).expect("original output directory"); let swapped_output = original.join("bundle.zip"); - let swap_original = original.clone(); + let swap_original = original; let swap_moved = moved.clone(); test_support::set( Stage::BeforePublish, @@ -933,7 +933,7 @@ mod tests { let moved = temp.path().join("publish-moved"); fs::create_dir(&original).expect("original publish directory"); let swapped_output = original.join("bundle.zip"); - let swap_original = original.clone(); + let swap_original = original; let swap_moved = moved.clone(); test_support::set( Stage::Rename, diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs index 8fc22cdcc..c033caeda 100644 --- a/rustfs/src/error.rs +++ b/rustfs/src/error.rs @@ -307,7 +307,7 @@ impl From for ApiError { StorageError::InvalidArgument(_, _, _) => S3ErrorCode::InvalidArgument, StorageError::MethodNotAllowed => S3ErrorCode::MethodNotAllowed, StorageError::BucketNotFound(_) => S3ErrorCode::NoSuchBucket, - StorageError::BucketNotEmpty(_) => S3ErrorCode::BucketNotEmpty, + StorageError::BucketNotEmpty(_) | StorageError::BucketNotEmptyWithDetails { .. } => S3ErrorCode::BucketNotEmpty, StorageError::BucketNameInvalid(_) => S3ErrorCode::InvalidBucketName, StorageError::ObjectNameInvalid(_, _) => S3ErrorCode::InvalidArgument, StorageError::BucketExists(_) => S3ErrorCode::BucketAlreadyOwnedByYou, @@ -706,6 +706,13 @@ mod tests { (StorageError::MethodNotAllowed, S3ErrorCode::MethodNotAllowed), (StorageError::BucketNotFound("test".into()), S3ErrorCode::NoSuchBucket), (StorageError::BucketNotEmpty("test".into()), S3ErrorCode::BucketNotEmpty), + ( + StorageError::BucketNotEmptyWithDetails { + bucket: "test".into(), + details: "metadata-less residue".into(), + }, + S3ErrorCode::BucketNotEmpty, + ), (StorageError::BucketNameInvalid("test".into()), S3ErrorCode::InvalidBucketName), ( StorageError::ObjectNameInvalid("test".into(), "test".into()),