fix(pools): preserve shared tier references during failed deletes

Reuse deleteObjectReconciled for tiered bulk entries and unversioned single deletes. Its reference-aware cleanup avoids scheduling remote contents for garbage collection while another copy still needs them, including restored copies. Keep the normal local-data bulk path unchanged.

Extend existing tier GC regressions to bulk and unversioned deletes and include them in the multi-pool race CI step.

Signed-off-by: Feng Ruohang <rh@vonng.com>

.github/workflows/go.yml

@@ -94,7 +94,7 @@ jobs:
         run: go test -race ./cmd -run '^Test(PoolsConditionalPut|SinglePoolConditionalPutHTTP)' -count=1 -timeout=5m
 
       - name: Run multi-pool delete consistency tests under race detector
-        run: go test -race ./cmd -run '^Test(Pools(Delete|BulkDelete)|BulkDeleteQuorumEvidence)' -count=1 -timeout=5m
+        run: go test -race ./cmd -run '^Test(Pools(Delete|BulkDelete|RetiringCopyPreservesSharedTierObject)|BulkDeleteQuorumEvidence)' -count=1 -timeout=5m
 
       - name: Run multipart listing, cancellation and cleanup tests under race detector
         env:

cmd/erasure-server-pool-consistency.go

@@ -542,6 +542,7 @@ func (z *erasureServerPools) deleteObjectsReconciled(ctx context.Context, bucket
 	deleted := make([]DeletedObject, len(objects))
 	errs := make([]error, len(objects))
 	primary := make([]int, len(objects))
+	tiered := make([]bool, len(objects))
 	lookup := errgroup.WithNErrs(len(objects)).WithConcurrency(32)
 	for i, obj := range objects {
 		// Preserve the full request identity even if lookup or cleanup fails.
@@ -556,6 +557,9 @@ func (z *erasureServerPools) deleteObjectsReconciled(ctx context.Context, bucket
 			copies, err := z.objectPoolInfos(ctx, bucket, obj.ObjectName, ObjectOptions{VersionID: obj.VersionID, NoAuditLog: true})
 			if err == nil {
 				primary[i] = copies[0].Index
+				for _, copy := range copies {
+					tiered[i] = tiered[i] || copy.ObjInfo.TransitionedObject.Status == lifecycle.TransitionComplete
+				}
 			} else if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
 				if isErrReadQuorum(err) {
 					err = InsufficientWriteQuorum{Bucket: bucket, Object: decodeDirObject(obj.ObjectName)}
@@ -576,7 +580,7 @@ func (z *erasureServerPools) deleteObjectsReconciled(ctx context.Context, bucket
 	for phase := 0; phase < 2; phase++ {
 		batches := make([]poolBatch, len(z.serverPools))
 		for i, obj := range objects {
-			if errs[i] != nil {
+			if errs[i] != nil || tiered[i] {
 				continue
 			}
 			for p := range z.serverPools {
@@ -614,5 +618,26 @@ func (z *erasureServerPools) deleteObjectsReconciled(ctx context.Context, bucket
 			}
 		}
 	}
+	// Tiered copies can share one remote object, including restored copies.
+	// Reuse the existing reconciliation which suppresses premature tier GC.
+	// Serializing this uncommon path also keeps duplicate key/version entries
+	// in one request from racing their metadata updates under the shared lock.
+	for i, obj := range objects {
+		if !tiered[i] || errs[i] != nil {
+			continue
+		}
+		deleteOpts := opts
+		deleteOpts.VersionID, deleteOpts.NoLock = obj.VersionID, true
+		deleteOpts.DeleteReplication = obj.ReplicationState()
+		if opts.PrefixEnabledFn != nil {
+			deleteOpts.Versioned = opts.PrefixEnabledFn(obj.ObjectName)
+		}
+		_, err := z.deleteObjectReconciled(ctx, bucket, obj.ObjectName, deleteOpts)
+		if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+			errs[i] = err
+		} else {
+			deleted[i].found = true
+		}
+	}
 	return deleted, errs
 }

cmd/erasure-server-pool-consistency_test.go

@@ -1071,6 +1071,8 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 		restored          bool
 		unconditional     bool
 		skipFreeVersion   bool
+		bulk              bool
+		unversioned       bool
 	}{
 		{name: "metadata-copy"},
 		{name: "restored-metadata-copy", restored: true},
@@ -1081,6 +1083,14 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 		{name: "ordinary-failed-primary-delete", deleting: true, unconditional: true, failPrimaryDelete: true},
 		{name: "ordinary-successful-delete", deleting: true, unconditional: true},
 		{name: "ordinary-skip-free-version", deleting: true, unconditional: true, skipFreeVersion: true},
+		{name: "bulk-failed-primary-delete", deleting: true, bulk: true, failPrimaryDelete: true},
+		{name: "bulk-restored-failed-primary-delete", deleting: true, bulk: true, restored: true, failPrimaryDelete: true},
+		{name: "bulk-successful-delete", deleting: true, bulk: true},
+		{name: "bulk-distinct-reference", deleting: true, bulk: true, differentRemote: true},
+		{name: "bulk-skip-free-version", deleting: true, bulk: true, skipFreeVersion: true},
+		{name: "unversioned-failed-primary-delete", deleting: true, unconditional: true, unversioned: true, failPrimaryDelete: true},
+		{name: "unversioned-successful-delete", deleting: true, unconditional: true, unversioned: true},
+		{name: "bulk-unversioned-failed-primary-delete", deleting: true, bulk: true, unversioned: true, failPrimaryDelete: true},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			z, bucket := consistencyPools(t)
@@ -1094,13 +1104,13 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 			if test.restored {
 				metadata[xhttp.AmzRestore] = completedRestoreObj(time.Now().Add(time.Hour)).String()
 			}
-			oi := putConsistencyObject(t, z, bucket, object, 0, "data", ObjectOptions{Versioned: true, UserDefined: metadata})
+			oi := putConsistencyObject(t, z, bucket, object, 0, "data", ObjectOptions{Versioned: !test.unversioned, UserDefined: metadata})
 			secondaryMetadata := maps.Clone(metadata)
 			if test.differentRemote {
 				secondaryMetadata[ReservedMetadataPrefixLower+TransitionedObjectName] = "other-remote-object"
 			}
 			putConsistencyObject(t, z, bucket, object, 1, "data", ObjectOptions{
-				Versioned: true, VersionID: oi.VersionID, MTime: oi.ModTime, UserDefined: secondaryMetadata,
+				Versioned: !test.unversioned, VersionID: oi.VersionID, MTime: oi.ModTime, UserDefined: secondaryMetadata,
 			})
 			current, err := z.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: oi.VersionID})
 			if err != nil || current.TransitionedObject.Status != "complete" || current.IsRemote() == test.restored {
@@ -1113,7 +1123,11 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 				set := z.serverPools[0].getHashedSet(object)
 				getDisks := set.getDisks
 				faulty := append([]StorageAPI(nil), getDisks()...)
+				var failed, attempted atomic.Int32
 				for i := range faulty {
+					if test.bulk {
+						faulty[i] = bulkDeleteFaultDisk{StorageAPI: faulty[i], bucket: bucket, object: object, mode: "eio", fail: true, failed: &failed, attempted: &attempted}
+					}
 					faulty[i] = consistencyDeleteFaultDisk{StorageAPI: faulty[i], bucket: bucket, object: object, version: oi.VersionID}
 				}
 				set.getDisks = func() []StorageAPI { return faulty }
@@ -1121,13 +1135,20 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 			}
 			if test.deleting {
 				opts := ObjectOptions{
-					Versioned: true, VersionID: oi.VersionID, SkipFreeVersion: test.skipFreeVersion,
+					Versioned: !test.unversioned, VersionID: oi.VersionID, SkipFreeVersion: test.skipFreeVersion,
 					CheckPrecondFn: func(info ObjectInfo) bool { return info.ETag != current.ETag },
 				}
 				if test.unconditional {
 					opts.CheckPrecondFn = nil
 				}
-				_, err := z.DeleteObject(t.Context(), bucket, object, opts)
+				var err error
+				if test.bulk {
+					opts.CheckPrecondFn = nil
+					_, errs := z.DeleteObjects(t.Context(), bucket, []ObjectToDelete{{ObjectV: ObjectV{ObjectName: object, VersionID: oi.VersionID}}}, opts)
+					err = errs[0]
+				} else {
+					_, err = z.DeleteObject(t.Context(), bucket, object, opts)
+				}
 				if (err != nil) != test.failPrimaryDelete {
 					t.Fatalf("unexpected authoritative delete result: %v", err)
 				}
@@ -1145,7 +1166,7 @@ func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
 			_, err = z.serverPools[0].GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: oi.VersionID})
 			primaryDeleted := test.deleting && !test.failPrimaryDelete
 			if primaryDeleted {
-				if !isErrVersionNotFound(err) {
+				if !isErrVersionNotFound(err) && (!test.unversioned || !isErrObjectNotFound(err)) {
 					t.Fatalf("authoritative copy survived successful delete: %v", err)
 				}
 			} else if err != nil {

cmd/erasure-server-pool.go

@@ -39,6 +39,7 @@ import (
 	"github.com/minio/minio-go/v7/pkg/set"
 	"github.com/minio/minio-go/v7/pkg/tags"
 	"github.com/minio/minio/internal/bpool"
+	"github.com/minio/minio/internal/bucket/lifecycle"
 	"github.com/minio/minio/internal/cachevalue"
 	"github.com/minio/minio/internal/config/storageclass"
 	xioutil "github.com/minio/minio/internal/ioutil"
@@ -1357,6 +1358,11 @@ func (z *erasureServerPools) DeleteObject(ctx context.Context, bucket string, ob
 
 	// Retire older copies before the newest copy for unversioned objects.
 	if len(noReadQuorumPools) > 0 && !opts.Versioned && !opts.VersionSuspended {
+		if pinfo.ObjInfo.TransitionedObject.Status == lifecycle.TransitionComplete {
+			// Restored and remote copies can share a tier object. Reuse the
+			// reconciliation which keeps that reference out of premature GC.
+			return z.deleteObjectReconciled(ctx, bucket, object, opts)
+		}
 		return z.deleteObjectFromAllPools(ctx, bucket, object, opts, noReadQuorumPools)
 	}