From 59db7d1e52d8c12e1a755f783119caa1514d2d84 Mon Sep 17 00:00:00 2001
From: Josh Siviour <mrk@mulgadc.com>
Date: Sun, 2 Aug 2026 08:18:27 +1000
Subject: [PATCH] fix: stop calling RemoveSystemImage's deleted bytes "freed"
 (#638)

BytesFreed counted logical object sizes deleted from the S3 namespace,
but predastore only reclaims that disk space later via background
compaction. The CLI printed "freed X GiB" immediately on delete, which
made a compaction outage that never reclaimed the space invisible.

Rename to BytesDeleted and reword the CLI output to say the bytes are
marked for deletion and reclaimed by background compaction, not freed
immediately.
---
 cmd/spinifex/cmd/admin.go            |  6 ++++--
 spinifex/admin/images_remove.go      | 15 ++++++++++-----
 spinifex/admin/images_remove_test.go |  2 +-
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go
index 750a96662..9253cfaea 100644
--- a/cmd/spinifex/cmd/admin.go
+++ b/cmd/spinifex/cmd/admin.go
@@ -903,8 +903,10 @@ func runimagesRemoveCmd(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 
-	fmt.Printf("✅ Removed AMI %s (freed %s across %d objects).\n",
-		imageID, utils.HumanBytes(utils.SafeInt64ToUint64(res.BytesFreed)), res.ObjectsDeleted)
+	// BytesDeleted is logical: predastore reclaims the underlying disk space
+	// asynchronously via background compaction, not at delete time.
+	fmt.Printf("✅ Removed AMI %s (%d objects, %s marked for deletion; disk space is reclaimed by background compaction).\n",
+		imageID, res.ObjectsDeleted, utils.HumanBytes(utils.SafeInt64ToUint64(res.BytesDeleted)))
 }
 
 func printDependents(w io.Writer, d admin.Dependents) {
diff --git a/spinifex/admin/images_remove.go b/spinifex/admin/images_remove.go
index e772ad6be..7c0e72efb 100644
--- a/spinifex/admin/images_remove.go
+++ b/spinifex/admin/images_remove.go
@@ -29,9 +29,14 @@ type RemoveImageOpts struct {
 }
 
 // RemoveImageResult summarises what was deleted from object storage.
+//
+// BytesDeleted is the logical size of the deleted objects, not physical disk
+// space: predastore reclaims a deleted object's bytes asynchronously via
+// background compaction, so this count can be freed well after the call
+// returns, or briefly not at all if compaction is stalled.
 type RemoveImageResult struct {
 	ObjectsDeleted int
-	BytesFreed     int64
+	BytesDeleted   int64
 }
 
 // Dependents lists every resource that transitively backs an admin-imported
@@ -259,7 +264,7 @@ func RemoveSystemImage(store objectstore.ObjectStore, bucket string, opts Remove
 			return nil, fmt.Errorf("delete config: %w", err)
 		}
 		result.ObjectsDeleted += n
-		result.BytesFreed += b
+		result.BytesDeleted += b
 	}
 
 	// Step 2: drop the rest of ami-<id>/ (chunks, WAL, checkpoints).
@@ -268,7 +273,7 @@ func RemoveSystemImage(store objectstore.ObjectStore, bucket string, opts Remove
 		return nil, fmt.Errorf("delete ami prefix: %w", err)
 	}
 	result.ObjectsDeleted += n
-	result.BytesFreed += b
+	result.BytesDeleted += b
 
 	// Step 3: drop snap-<amiID>/ (the viperblock-internal snap checkpoint).
 	n, b, err = deletePrefix(store, bucket, SnapPrefix(opts.ImageID)+"/")
@@ -276,12 +281,12 @@ func RemoveSystemImage(store objectstore.ObjectStore, bucket string, opts Remove
 		return nil, fmt.Errorf("delete snap prefix: %w", err)
 	}
 	result.ObjectsDeleted += n
-	result.BytesFreed += b
+	result.BytesDeleted += b
 
 	slog.Info("RemoveSystemImage completed",
 		"imageId", opts.ImageID,
 		"objectsDeleted", result.ObjectsDeleted,
-		"bytesFreed", result.BytesFreed,
+		"bytesDeleted", result.BytesDeleted,
 		"force", opts.Force,
 	)
 	return result, nil
diff --git a/spinifex/admin/images_remove_test.go b/spinifex/admin/images_remove_test.go
index 44bc8e96d..8f69cbb10 100644
--- a/spinifex/admin/images_remove_test.go
+++ b/spinifex/admin/images_remove_test.go
@@ -128,7 +128,7 @@ func TestRemoveSystemImage_HappyPath(t *testing.T) {
 	res, err := RemoveSystemImage(store, testRemoveBucket, RemoveImageOpts{ImageID: id})
 	require.NoError(t, err)
 	assert.Equal(t, preview.AMIObjectCount+preview.SnapObjectCount, res.ObjectsDeleted)
-	assert.Equal(t, preview.AMIBytesTotal+preview.SnapBytesTotal, res.BytesFreed)
+	assert.Equal(t, preview.AMIBytesTotal+preview.SnapBytesTotal, res.BytesDeleted)
 	assert.Equal(t, 0, store.Count())
 }