fix(ecstore): roll back delete on disks that staged then errored (#4676)

fix(ecstore): roll back delete on disks that staged then errored (backlog#1158)

#4300 rolls back a failed delete only on disks that returned Ok, skipping any
disk that staged its rollback backup, applied the delete, and then errored --
leaving that disk deleted while its peers are restored. On rollback, fan the
undo out to every online disk instead; the disk-side restore_delete_rollback
is already idempotent (Ok no-op when nothing was staged), so unstaged disks
are unaffected. The err.is_some() skip now applies only to the success/cleanup
path. Covers both single-object and batch delete.

Refs backlog#1158.

crates/ecstore/src/disk/local.rs

@@ -9956,6 +9956,66 @@ mod test {
         assert!(!object_dir.join(rollback_dir.to_string()).exists());
     }
 
+    // backlog#1158 safety premise: the set layer fans the undo out to every online
+    // disk on a quorum-failed delete, including disks that never staged a rollback
+    // (e.g. a disk that errored before staging). A delete-undo on such a disk must be
+    // a safe no-op that leaves the committed object untouched.
+    #[tokio::test]
+    async fn test_delete_version_undo_is_noop_when_nothing_staged() {
+        use tempfile::tempdir;
+
+        let dir = tempdir().expect("temp dir should be created");
+        let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse");
+        let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created");
+
+        let bucket = "bucket";
+        let object = "dir/object";
+        let version_id = Uuid::parse_str("aaaaaaaa-1111-2222-3333-444444444444").expect("version id should parse");
+        let data_dir = Uuid::parse_str("bbbbbbbb-1111-2222-3333-444444444444").expect("data dir should parse");
+        let rollback_dir = Uuid::parse_str("cccccccc-1111-2222-3333-444444444444").expect("rollback dir should parse");
+
+        ensure_test_volume(&disk, bucket).await;
+
+        // A committed object exists on this disk, but no rollback state was staged.
+        let object_dir = dir.path().join(bucket).join("dir/object");
+        let data_path = object_dir.join(data_dir.to_string());
+        fs::create_dir_all(&data_path).await.expect("data dir should be created");
+        fs::write(data_path.join("part.1"), b"live-data")
+            .await
+            .expect("part data should be written");
+        let fi = test_file_info(object, version_id, Some(data_dir), None);
+        let meta = test_meta(fi.clone());
+        fs::write(object_dir.join(STORAGE_FORMAT_FILE), meta.clone())
+            .await
+            .expect("metadata should be written");
+
+        // Undo targeting a rollback dir that was never created must be an Ok no-op.
+        disk.delete_version(
+            bucket,
+            object,
+            fi.clone(),
+            false,
+            DeleteOptions {
+                undo_write: true,
+                undo_delete: true,
+                old_data_dir: Some(rollback_dir),
+                ..Default::default()
+            },
+        )
+        .await
+        .expect("undo with no staged rollback state must be a no-op");
+
+        // The committed object is untouched.
+        assert_eq!(
+            fs::read(object_dir.join(STORAGE_FORMAT_FILE))
+                .await
+                .expect("metadata should remain"),
+            meta
+        );
+        assert_eq!(fs::read(data_path.join("part.1")).await.expect("data should remain"), b"live-data");
+        assert!(!object_dir.join(rollback_dir.to_string()).exists());
+    }
+
     #[tokio::test]
     async fn test_delete_marker_rollback_removes_new_metadata_without_backup() {
         use tempfile::tempdir;

crates/ecstore/src/set_disk/ops/object.rs

@@ -1409,7 +1409,14 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
         let should_rollback = quorum_result.is_err();
         let mut rollback_futures = Vec::new();
         for (index, err) in errs.iter().enumerate() {
-            if err.is_some() {
+            // backlog#1158: when rolling back, fan the idempotent undo out to every
+            // online disk (each self-decides from its staged backup: restore if the
+            // rollback dir is present, no-op otherwise). This covers a disk that
+            // staged + applied the delete and *then* errored, which the plain
+            // `err.is_some()` skip would leave deleted while its peers were restored.
+            // On success only the successful disks' backup dirs need cleaning; errored
+            // disks' residue is reclaimed by heal/scanner.
+            if !should_rollback && err.is_some() {
                 continue;
             }
 
@@ -1764,7 +1771,11 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
             // delete_versions commits one xl.meta per object group, so rollback must use the same boundary.
             let should_rollback = fi_vers.versions.iter().any(|fi| del_errs[fi.idx].is_some());
             for (disk_idx, disk) in disks.iter().enumerate() {
-                if fi_vers.versions.iter().any(|fi| del_obj_errs[disk_idx][fi.idx].is_some()) {
+                // backlog#1158: on rollback, include every online disk so a disk that
+                // staged + applied the delete and then errored is still restored (the
+                // disk-side undo is idempotent, no-op when nothing was staged). On
+                // success, skip the errored disks and only clean up successful ones.
+                if !should_rollback && fi_vers.versions.iter().any(|fi| del_obj_errs[disk_idx][fi.idx].is_some()) {
                     continue;
                 }