From 7b1ffbfdc1f54e24f9de082f586ed28f119439f3 Mon Sep 17 00:00:00 2001 From: overtrue Date: Sun, 4 Oct 2026 07:35:00 +0800 Subject: [PATCH] fix(ecstore): hide rollback backup residue in prefix listings --- crates/ecstore/src/disk/local.rs | 60 +++++++++++++++++++++--- crates/ecstore/src/store/list_objects.rs | 29 +++++++++++- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/crates/ecstore/src/disk/local.rs b/crates/ecstore/src/disk/local.rs index 124799644..1e41c2dce 100644 --- a/crates/ecstore/src/disk/local.rs +++ b/crates/ecstore/src/disk/local.rs @@ -165,12 +165,13 @@ fn is_delete_transaction_marker(entry: &str, prefix: &str) -> bool { .is_some_and(|transaction| Uuid::parse_str(transaction).is_ok_and(|uuid| !uuid.is_nil())) } -/// Whether a `list_dir` entry inside a UUID data dir is erasure data or a -/// delete-transaction marker. Anything else (a subdirectory, an `xl.meta`, an -/// unknown file) means the directory is not plain delete residue. +/// Whether a `list_dir` entry inside a UUID data dir is erasure data, a rollback +/// metadata backup, or a delete-transaction marker. Anything else (a subdirectory, +/// an `xl.meta`, an unknown file) means the directory is not plain delete residue. fn is_metadata_less_data_dir_entry(entry: &str) -> bool { !entry.ends_with(SLASH_SEPARATOR) && (metadata_less_part_file(entry) + || entry == STORAGE_FORMAT_FILE_BACKUP || is_delete_transaction_marker(entry, DELETE_DATA_DIR_MARKER_PREFIX) || is_delete_transaction_marker(entry, RESERVED_DELETE_DATA_DIR_MARKER_PREFIX)) } @@ -8496,9 +8497,9 @@ impl LocalDisk { /// Whether the metadata-less directory `dir_name` holds nothing that a /// listing could show: every leaf under it is the data dir of a deleted - /// version (a non-nil UUID directory of `part.N` files and - /// delete-transaction markers) or an empty directory. That is what an - /// interrupted or deferred version delete leaves behind once the `xl.meta` + /// version (a non-nil UUID directory of `part.N` files, rollback metadata + /// backups, and delete-transaction markers) or an empty directory. That is + /// what an interrupted or deferred version delete leaves behind once the `xl.meta` /// is gone, and neither the object directory nor the date-style ancestors /// above it may surface as prefixes (#6898). Real objects are directories /// carrying their own `xl.meta`, so the first file met outside a UUID data @@ -19622,13 +19623,42 @@ mod test { .await .expect("delete marker should be written"); + // Inline deletes can leave only a rollback backup; non-inline deletes + // can leave the backup alongside parts. Neither has listable metadata. + for (prefix, has_parts) in [("backup-only", false), ("backup-parts", true)] { + let backup = bucket_dir + .join(prefix) + .join("2026/08/28/23/object.parquet") + .join(Uuid::new_v4().to_string()); + fs::create_dir_all(&backup).await.expect("backup residue should be created"); + fs::write(backup.join(STORAGE_FORMAT_FILE_BACKUP), b"rollback metadata") + .await + .expect("rollback backup should be written"); + if has_parts { + fs::write(backup.join("part.1"), b"stale") + .await + .expect("stale part beside backup should be written"); + } + } + // A user prefix made of UUID-named directories holding real objects. let upload = Uuid::new_v4().to_string(); write_object(&bucket_dir.join("uploads").join(&upload).join("file"), &format!("uploads/{upload}/file")).await; + write_object( + &bucket_dir.join("uploads").join(&upload).join(STORAGE_FORMAT_FILE_BACKUP), + &format!("uploads/{upload}/{STORAGE_FORMAT_FILE_BACKUP}"), + ) + .await; // An object whose key is itself a UUID. let named = Uuid::new_v4().to_string(); write_object(&bucket_dir.join("named").join(&named), &format!("named/{named}")).await; + fs::write( + bucket_dir.join("named").join(&named).join(STORAGE_FORMAT_FILE_BACKUP), + b"rollback metadata", + ) + .await + .expect("backup beside live metadata should be written"); // Residue next to a live child object. let mixed_residue = bucket_dir.join("mixed").join(Uuid::new_v4().to_string()); @@ -19685,10 +19715,25 @@ mod test { assert_eq!(scan_names(&disk, bucket, "residue/2026/").await, Vec::::new()); assert_eq!(scan_names(&disk, bucket, "residue/").await, Vec::::new()); assert_eq!(scan_names(&disk, bucket, "committed/").await, Vec::::new()); + for prefix in ["backup-only", "backup-parts"] { + assert_eq!( + scan_names(&disk, bucket, &format!("{prefix}/2026/08/28/23/")).await, + Vec::::new(), + "rollback backups must not surface as object prefixes" + ); + assert_eq!(scan_names(&disk, bucket, &format!("{prefix}/")).await, Vec::::new()); + } // UUID-named directories holding real objects, an object keyed by a // UUID, and residue beside a live child all remain visible. assert_eq!(scan_names(&disk, bucket, "uploads/").await, vec![format!("uploads/{upload}/")]); + assert_eq!( + scan_names(&disk, bucket, &format!("uploads/{upload}/")).await, + vec![ + format!("uploads/{upload}/file"), + format!("uploads/{upload}/{STORAGE_FORMAT_FILE_BACKUP}") + ] + ); assert_eq!(scan_names(&disk, bucket, "named/").await, vec![format!("named/{named}")]); assert_eq!(scan_names(&disk, bucket, "mixed/").await, vec!["mixed/child".to_owned()]); assert_eq!( @@ -19778,6 +19823,9 @@ mod test { fs::write(bucket_dir.join("stray/2026/08/notes.txt"), b"?") .await .expect("stray file should be written"); + fs::write(bucket_dir.join("stray/2026/08").join(STORAGE_FORMAT_FILE_BACKUP), b"?") + .await + .expect("backup outside a UUID data directory should be written"); // A UUID directory holding a subdirectory is not a data dir shape the // probe understands, so it surfaces even when the subdirectory is empty. diff --git a/crates/ecstore/src/store/list_objects.rs b/crates/ecstore/src/store/list_objects.rs index 6ecabdad2..07ef31352 100644 --- a/crates/ecstore/src/store/list_objects.rs +++ b/crates/ecstore/src/store/list_objects.rs @@ -10125,6 +10125,7 @@ mod test { .expect("bucket should be created with authoritative metadata"); let committed_dir = uuid::Uuid::new_v4(); let unmarked_dir = uuid::Uuid::new_v4(); + let backup_dir = uuid::Uuid::new_v4(); let transaction = uuid::Uuid::new_v4(); for dir in &dirs { let committed = dir @@ -10135,6 +10136,9 @@ mod test { tokio::fs::create_dir_all(&committed) .await .expect("committed delete residue should be created"); + tokio::fs::write(committed.join(crate::disk::STORAGE_FORMAT_FILE_BACKUP), b"rollback metadata") + .await + .expect("committed rollback backup should be written"); tokio::fs::write(committed.join("part.1"), b"stale") .await .expect("stale part should be written"); @@ -10158,6 +10162,18 @@ mod test { tokio::fs::write(unmarked.join("part.1"), b"stale") .await .expect("stale part should be written"); + + let backup = dir + .path() + .join(bucket) + .join("metrics/statefulset/2026/08/26/18/object.parquet") + .join(backup_dir.to_string()); + tokio::fs::create_dir_all(&backup) + .await + .expect("rollback backup directory should be created"); + tokio::fs::write(backup.join(crate::disk::STORAGE_FORMAT_FILE_BACKUP), b"rollback metadata") + .await + .expect("rollback backup should be written"); } let result = store @@ -10166,7 +10182,7 @@ mod test { .await .expect("delimiter listing should succeed"); assert!(result.objects.is_empty()); - assert!(result.prefixes.is_empty(), "neither residue subtree may surface as a prefix"); + assert!(result.prefixes.is_empty(), "residue subtrees may not surface as prefixes"); for dir in &dirs { let metrics = dir.path().join(bucket).join("metrics"); assert!( @@ -10181,6 +10197,17 @@ mod test { .exists(), "residue without a committed marker must survive the purge" ); + assert_eq!( + tokio::fs::read( + metrics + .join("statefulset/2026/08/26/18/object.parquet") + .join(backup_dir.to_string()) + .join(crate::disk::STORAGE_FORMAT_FILE_BACKUP) + ) + .await + .expect("uncommitted rollback backup must survive the listing"), + b"rollback metadata" + ); } }