mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-03 18:55:39 +00:00
PR #4220 added a purge for orphan empty-directory trees (folder keys with no xl.meta) on the delete path, but the guard only accepted object-not-found. Over the real HTTP DELETE path the guard is never reached: `del_opts` pins `version_id = Uuid::nil()` for directory keys, so the missing dir object fails the specific-version lookup with version-not-found (FileVersionNotFound), not object-not-found. The guard short-circuits, the store returns the error, and the API layer turns it into a fake 204 — the ghost folder survives, exactly the #4189 symptom. The existing unit tests passed because they call `purge_orphan_dir_object` directly, bypassing the nil-version lookup. Accept both misses in the guard (extracted as `should_purge_orphan_dir_on_missing`) so the folder delete actually takes effect. Non-directory keys and non-miss errors (e.g. quorum failures) are unaffected; the cross-disk data-safety refusal in `purge_orphan_dir_object` is unchanged. Verified end-to-end against a running 4-disk erasure server: DELETE of a planted orphan `ghost/` tree now purges it on all drives and clears the listing, a real object under the same prefix is untouched, and a prefix holding real data on any drive is refused (all drives preserved). Adds predicate regression tests covering version-not-found / object-not-found on directory keys, and the negative cases.
This commit is contained in:
@@ -220,6 +220,18 @@ fn should_create_delete_marker_for_missing_object(opts: &ObjectOptions) -> bool
|
|||||||
opts.versioned && opts.version_id.is_none() && !opts.delete_marker && !opts.data_movement
|
opts.versioned && opts.version_id.is_none() && !opts.delete_marker && !opts.data_movement
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether a delete-time lookup miss on a directory key should trigger an orphan
|
||||||
|
/// empty-directory tree purge (issue #4189).
|
||||||
|
///
|
||||||
|
/// The lookup surfaces *version*-not-found here, not object-not-found: `del_opts`
|
||||||
|
/// pins `version_id = Uuid::nil()` for directory keys, so a missing dir object fails
|
||||||
|
/// the specific-version lookup. Both misses must be accepted, otherwise the real
|
||||||
|
/// HTTP delete path (which always sets the nil version) never reaches the purge and
|
||||||
|
/// the ghost folder survives with a fake 204 — the exact #4189 symptom.
|
||||||
|
fn should_purge_orphan_dir_on_missing(err: &Error, object: &str) -> bool {
|
||||||
|
(is_err_object_not_found(err) || is_err_version_not_found(err)) && rustfs_utils::path::is_dir_object(object)
|
||||||
|
}
|
||||||
|
|
||||||
fn version_aware_lookup_opts(opts: &ObjectOptions, no_lock: bool) -> ObjectOptions {
|
fn version_aware_lookup_opts(opts: &ObjectOptions, no_lock: bool) -> ObjectOptions {
|
||||||
let mut lookup_opts = opts.clone();
|
let mut lookup_opts = opts.clone();
|
||||||
lookup_opts.no_lock = no_lock;
|
lookup_opts.no_lock = no_lock;
|
||||||
@@ -957,10 +969,7 @@ impl ECStore {
|
|||||||
// disk as an orphan empty-directory tree (issue #4189): listings show
|
// disk as an orphan empty-directory tree (issue #4189): listings show
|
||||||
// it as a common prefix, but no regular delete path can remove it.
|
// it as a common prefix, but no regular delete path can remove it.
|
||||||
// Purge the orphan tree so folder deletes actually take effect.
|
// Purge the orphan tree so folder deletes actually take effect.
|
||||||
if is_err_object_not_found(&err)
|
if should_purge_orphan_dir_on_missing(&err, object) && self.purge_orphan_dir_object(bucket, object).await {
|
||||||
&& rustfs_utils::path::is_dir_object(object)
|
|
||||||
&& self.purge_orphan_dir_object(bucket, object).await
|
|
||||||
{
|
|
||||||
return Ok(ObjectInfo {
|
return Ok(ObjectInfo {
|
||||||
bucket: bucket.to_owned(),
|
bucket: bucket.to_owned(),
|
||||||
name: decode_dir_object(object),
|
name: decode_dir_object(object),
|
||||||
@@ -1775,6 +1784,43 @@ mod tests {
|
|||||||
assert!(!should_create_delete_marker_for_missing_object(&data_movement));
|
assert!(!should_create_delete_marker_for_missing_object(&data_movement));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// issue #4189 regression: `del_opts` pins `version_id = Uuid::nil()` on directory
|
||||||
|
// keys, so deleting a ghost folder over HTTP fails the lookup with *version*-not-found
|
||||||
|
// (not object-not-found). The orphan-purge guard must accept both misses, or the
|
||||||
|
// ghost tree survives behind a fake 204 — the exact reported symptom.
|
||||||
|
#[test]
|
||||||
|
fn should_purge_orphan_dir_on_version_not_found_for_dir_key() {
|
||||||
|
assert!(
|
||||||
|
should_purge_orphan_dir_on_missing(&StorageError::FileVersionNotFound, "ghost/"),
|
||||||
|
"the real HTTP delete path yields version-not-found on dir keys and must reach the purge"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
should_purge_orphan_dir_on_missing(
|
||||||
|
&StorageError::VersionNotFound("bucket".into(), "ghost/".into(), Uuid::nil().to_string()),
|
||||||
|
"ghost/"
|
||||||
|
),
|
||||||
|
"typed VersionNotFound on a dir key must also reach the purge"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn should_purge_orphan_dir_on_object_not_found_for_dir_key() {
|
||||||
|
assert!(should_purge_orphan_dir_on_missing(&StorageError::FileNotFound, "ghost/"));
|
||||||
|
assert!(should_purge_orphan_dir_on_missing(
|
||||||
|
&StorageError::ObjectNotFound("bucket".into(), "ghost/".into()),
|
||||||
|
"ghost/"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn should_not_purge_orphan_dir_for_regular_key_or_other_errors() {
|
||||||
|
// A regular (non-directory) key must never trigger a prefix purge, even on a miss.
|
||||||
|
assert!(!should_purge_orphan_dir_on_missing(&StorageError::FileVersionNotFound, "regular.txt"));
|
||||||
|
assert!(!should_purge_orphan_dir_on_missing(&StorageError::FileNotFound, "regular.txt"));
|
||||||
|
// Non-miss errors (e.g. quorum failures) must not be masked by a purge attempt.
|
||||||
|
assert!(!should_purge_orphan_dir_on_missing(&StorageError::ErasureReadQuorum, "ghost/"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn resolve_decommission_target_pool_idx_result_passthrough_ok() {
|
fn resolve_decommission_target_pool_idx_result_passthrough_ok() {
|
||||||
let idx = ECStore::resolve_decommission_target_pool_idx_result(Ok(3), "bucket", "object").unwrap();
|
let idx = ECStore::resolve_decommission_target_pool_idx_result(Ok(3), "bucket", "object").unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user