fix(ecstore): purge orphan empty-directory trees on folder delete (#4220)

fix(ecstore): purge orphan empty-directory trees on folder delete (#4189)

Empty "phantom" folders — on-disk directory trees with no xl.meta
anywhere — show up in listings as common prefixes but cannot be removed
by any S3 call: DeleteObject on the folder key encodes it to __XLDIR__,
finds no metadata, and returns NotFound, which the API layer masks as a
fake 204 while the tree survives on disk.

Make handle_delete_object attempt a guarded purge when a dir-object key
resolves to NotFound:

- sweep every erasure set of every pool (orphan fragments can sit on
  any set, left behind by whichever sets stored the deleted children)
- per set, refuse if ANY disk holds a regular file under the prefix,
  so degraded/healable objects are never destroyed
- remove empty directories children-first with non-recursive deletes;
  a racing PutObject makes the rmdir fail with DirectoryNotEmpty and
  the entry is skipped

This state is unrepresentable on AWS S3 (CommonPrefixes derive only
from real keys; DeleteObject is an idempotent 204), so the purge moves
visible behavior closer to S3. It deliberately goes beyond MinIO, whose
DeleteObject leaves these trees stranded and whose heal only removes
dangling (minority-presence) dirs — a long-standing complaint
(minio/minio#15257, #19516, #18444).

Also move the test-only content_matches_by_etag import in
replication_target_boundary.rs into the cfg(test) module; it failed
cargo clippy -D warnings and blocked the pre-PR gate.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Zhengchao An
2026-07-03 11:34:43 +08:00
committed by GitHub
parent fb0e2d6d8f
commit e9af89b40d
3 changed files with 304 additions and 1 deletions
+134
View File
@@ -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<Option<DiskStore>>) -> Arc<SetDisks> {
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::<Vec<_>>();
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() {
+125
View File
@@ -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<String>),
/// 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<String> = 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<bool> {
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<String>)> = 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,
+45 -1
View File
@@ -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<ObjectInfo> {
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() {