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.
This commit is contained in:
Josh Siviour
2026-08-02 08:18:27 +10:00
committed by GitHub
parent 9f5f877e50
commit 59db7d1e52
3 changed files with 15 additions and 8 deletions
+4 -2
View File
@@ -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) {
+10 -5
View File
@@ -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
+1 -1
View File
@@ -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())
}