diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index 5d68ccf76..e1f6e5413 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -7869,6 +7869,140 @@ mod tests { assert!(result.is_err(), "exact prefix delete should wait on the real object namespace lock"); } + async fn make_single_local_disk() -> (TempDir, DiskStore) { + let dir = tempfile::tempdir().expect("tempdir should be created"); + let endpoint = + Endpoint::try_from(dir.path().to_str().expect("tempdir path should be utf8")).expect("endpoint should parse"); + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("disk should be created"); + (dir, disk) + } + + async fn make_set_disks_with(disks: Vec>) -> Arc { + let drive_count = disks.len(); + let endpoints = (0..drive_count) + .map(|i| Endpoint::try_from(format!("http://127.0.0.1:{}/data", 9000 + i).as_str()).expect("endpoint should parse")) + .collect::>(); + + SetDisks::new( + "test-owner".to_string(), + Arc::new(RwLock::new(disks)), + drive_count, + 0, + 0, + 0, + endpoints, + FormatV3::new(1, drive_count), + vec![Arc::new(LocalClient::with_manager(Arc::new( + rustfs_lock::GlobalLockManager::new(), + )))], + ) + .await + } + + // issue #4189: an orphan directory tree (empty dirs, no xl.meta) must be purged. + #[tokio::test] + async fn purge_orphan_dir_object_removes_empty_tree() { + let (dir, disk) = make_single_local_disk().await; + let root = dir.path(); + fs::create_dir_all(root.join("bucket").join("pfx").join("a").join("b")) + .await + .expect("nested empty dir should be created"); + fs::create_dir_all(root.join("bucket").join("pfx").join("c")) + .await + .expect("sibling empty dir should be created"); + + let set = make_set_disks_with(vec![Some(disk)]).await; + let purged = set + .purge_orphan_dir_object("bucket", "pfx/") + .await + .expect("purge should succeed"); + + assert!(purged, "orphan empty tree should be purged"); + assert!(!root.join("bucket").join("pfx").exists(), "prefix directory should be gone"); + assert!(root.join("bucket").exists(), "bucket volume should remain"); + } + + // issue #4189: a prefix that still anchors a real object must be left intact. + #[tokio::test] + async fn purge_orphan_dir_object_preserves_prefix_with_object() { + let (dir, disk) = make_single_local_disk().await; + let root = dir.path(); + let obj_dir = root.join("bucket").join("pfx").join("obj"); + fs::create_dir_all(&obj_dir).await.expect("object dir should be created"); + fs::write(obj_dir.join(STORAGE_FORMAT_FILE), b"meta") + .await + .expect("object metadata should be written"); + + let set = make_set_disks_with(vec![Some(disk)]).await; + let purged = set + .purge_orphan_dir_object("bucket", "pfx/") + .await + .expect("scan should succeed"); + + assert!(!purged, "prefix containing an object must not be purged"); + assert!(obj_dir.join(STORAGE_FORMAT_FILE).exists(), "object metadata must be preserved"); + } + + #[tokio::test] + async fn purge_orphan_dir_object_missing_returns_false() { + let (dir, disk) = make_single_local_disk().await; + fs::create_dir_all(dir.path().join("bucket")) + .await + .expect("bucket volume should be created"); + + let set = make_set_disks_with(vec![Some(disk)]).await; + let purged = set + .purge_orphan_dir_object("bucket", "does-not-exist/") + .await + .expect("scan should succeed"); + + assert!(!purged, "a missing prefix should report nothing to purge"); + } + + // Cross-disk safety: if any drive still holds object data under the prefix, refuse + // to purge on every drive so a degraded/healable object is never destroyed. + #[tokio::test] + async fn purge_orphan_dir_object_refuses_when_any_disk_has_data() { + let (dir0, disk0) = make_single_local_disk().await; + let (dir1, disk1) = make_single_local_disk().await; + + fs::create_dir_all(dir0.path().join("bucket").join("pfx").join("a")) + .await + .expect("disk0 empty tree should be created"); + + let obj_dir = dir1.path().join("bucket").join("pfx").join("a"); + fs::create_dir_all(&obj_dir) + .await + .expect("disk1 object dir should be created"); + fs::write(obj_dir.join(STORAGE_FORMAT_FILE), b"meta") + .await + .expect("disk1 object metadata should be written"); + + let set = make_set_disks_with(vec![Some(disk0), Some(disk1)]).await; + let purged = set + .purge_orphan_dir_object("bucket", "pfx/") + .await + .expect("scan should succeed"); + + assert!(!purged, "must not purge when any disk holds object data"); + assert!( + obj_dir.join(STORAGE_FORMAT_FILE).exists(), + "object metadata on the healthy disk must be preserved" + ); + assert!( + dir0.path().join("bucket").join("pfx").join("a").exists(), + "empty tree must be left untouched when the purge is aborted" + ); + } + #[tokio::test(flavor = "multi_thread")] #[serial] async fn test_acquire_dist_delete_object_locks_batch_succeeds_with_two_healthy_lockers() { diff --git a/crates/ecstore/src/set_disk/write.rs b/crates/ecstore/src/set_disk/write.rs index 03b833d43..b4803c4a9 100644 --- a/crates/ecstore/src/set_disk/write.rs +++ b/crates/ecstore/src/set_disk/write.rs @@ -14,6 +14,19 @@ use super::*; +/// Result of scanning one disk's copy of a directory prefix while deciding +/// whether an orphan (metadata-less) directory tree can be safely purged. +enum OrphanDirScan { + /// The subtree holds at least one regular file (object metadata or data), so + /// it is a real object and must not be purged. + HasData, + /// The prefix exists on this disk and contains only nested empty directories. + /// Carries every directory path in pre-order (parents before children). + Empty(Vec), + /// The prefix does not exist on this disk. + Missing, +} + impl SetDisks { pub(super) fn default_read_quorum(&self) -> usize { self.set_drive_count - self.default_parity_count @@ -712,6 +725,118 @@ impl SetDisks { Ok(()) } + /// Scan a single disk's copy of `prefix` and decide whether it is an orphan + /// (metadata-less) directory subtree. Walks the tree iteratively and returns + /// [`OrphanDirScan::HasData`] as soon as any regular file is found. + async fn scan_orphan_dir(disk: &DiskStore, bucket: &str, prefix: &str) -> OrphanDirScan { + let root = prefix.trim_end_matches(SLASH_SEPARATOR).to_string(); + let mut stack = vec![root.clone()]; + // Pre-order list of directories (a parent always precedes its descendants), + // so reversing it yields a safe children-first removal order. + let mut dirs: Vec = Vec::new(); + let mut existed = false; + + while let Some(dir) = stack.pop() { + let entries = match disk.list_dir("", bucket, &dir, 0).await { + Ok(entries) => entries, + Err(_) => { + // The root missing (or never existing) means there is nothing to + // purge on this disk. A nested directory vanishing mid-scan is a + // benign race, so skip it and keep walking. + if dir == root { + return OrphanDirScan::Missing; + } + continue; + } + }; + + existed = true; + dirs.push(dir.clone()); + + for entry in entries { + match entry.strip_suffix(SLASH_SEPARATOR) { + // `read_dir` marks directories with a trailing slash; anything else + // is a regular file, which means real object data lives here. + Some(child) => stack.push(format!("{dir}{SLASH_SEPARATOR}{child}")), + None => return OrphanDirScan::HasData, + } + } + } + + if existed { + OrphanDirScan::Empty(dirs) + } else { + OrphanDirScan::Missing + } + } + + /// Purge an orphan directory prefix — a trailing-slash key that exists on disk + /// as an empty directory tree with no object metadata on any disk of this set. + /// Such prefixes are listable (see `scan_dir`) yet are not real objects, so the + /// normal delete path returns NotFound and leaves them stranded (issue #4189). + /// + /// Callers pass the *decoded* directory name (`prefix/`), not the `__XLDIR__` + /// encoded object key — the orphan tree on disk uses the plain path. + /// + /// Returns `Ok(true)` when the prefix was an orphan tree on this set and was + /// removed, `Ok(false)` when it holds real data or does not exist on any disk + /// of this set (the caller should surface the original NotFound), and `Err` on + /// a hard disk failure. + pub(crate) async fn purge_orphan_dir_object(&self, bucket: &str, object: &str) -> disk::error::Result { + let disks = self.get_disks_internal().await; + + // Phase 1: classify every online disk. Refuse to purge if ANY disk holds + // object data under the prefix, so a degraded/healable object is never + // destroyed. + let mut per_disk_dirs: Vec<(usize, Vec)> = Vec::new(); + let mut existed = false; + for (i, disk) in disks.iter().enumerate() { + let Some(disk) = disk else { continue }; + match Self::scan_orphan_dir(disk, bucket, object).await { + OrphanDirScan::HasData => return Ok(false), + OrphanDirScan::Empty(dirs) => { + existed = true; + per_disk_dirs.push((i, dirs)); + } + OrphanDirScan::Missing => {} + } + } + + if !existed { + return Ok(false); + } + + // Phase 2: remove the empty directories children-first on each disk. A + // non-recursive delete performs an empty-only `rmdir`, so a directory that + // concurrently gained an object fails with DirectoryNotEmpty and is skipped — + // a racing PutObject is never clobbered. + for (i, mut dirs) in per_disk_dirs { + let Some(disk) = disks[i].as_ref() else { continue }; + dirs.reverse(); + for dir in dirs { + if let Err(err) = disk + .delete( + bucket, + &dir, + DeleteOptions { + recursive: false, + immediate: true, + ..Default::default() + }, + ) + .await + { + // Best effort: a sibling removal may have already cleared a shared + // parent, or a concurrent writer repopulated the directory. Neither + // is fatal to purging the orphan tree. + debug!(bucket, object, dir, error = ?err, "purge_orphan_dir_object: skipped non-empty/absent directory"); + } + } + } + + Ok(true) + } + pub(super) async fn check_write_precondition( &self, bucket: &str, diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 094c3fb8e..1ab4f0922 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -814,6 +814,34 @@ impl ECStore { )) } + /// Best-effort purge of an orphan directory prefix — an on-disk tree of empty + /// directories with no `xl.meta` anywhere (issue #4189). Orphan fragments can sit + /// on any erasure set of any pool (they are left behind by whichever sets stored + /// the now-deleted children), so every set is swept. Returns true when at least + /// one set removed an orphan tree. Hard per-set failures are logged and skipped: + /// the caller falls back to surfacing the original NotFound. + async fn purge_orphan_dir_object(&self, bucket: &str, object: &str) -> bool { + let prefix = decode_dir_object(object); + let mut purged = false; + for pool in self.pools.iter() { + for set in pool.disk_set.iter() { + match set.purge_orphan_dir_object(bucket, &prefix).await { + Ok(set_purged) => purged |= set_purged, + Err(err) => { + warn!( + bucket, + prefix, + pool_index = pool.pool_idx, + error = ?err, + "failed to purge orphan directory prefix" + ); + } + } + } + } + purged + } + #[instrument(skip(self))] pub(super) async fn handle_delete_object(&self, bucket: &str, object: &str, opts: ObjectOptions) -> Result { check_del_obj_args(bucket, object)?; @@ -905,7 +933,23 @@ impl ECStore { obj.name = decode_dir_object(object); return Ok(obj); } - Err(err) => return Err(err), + Err(err) => { + // A folder key (`prefix/`) with no object metadata may still exist on + // disk as an orphan empty-directory tree (issue #4189): listings show + // it as a common prefix, but no regular delete path can remove it. + // Purge the orphan tree so folder deletes actually take effect. + if is_err_object_not_found(&err) + && rustfs_utils::path::is_dir_object(object) + && self.purge_orphan_dir_object(bucket, object).await + { + return Ok(ObjectInfo { + bucket: bucket.to_owned(), + name: decode_dir_object(object), + ..Default::default() + }); + } + return Err(err); + } }; if pinfo.object_info.delete_marker && opts.version_id.is_none() {