fix(storage): preserve shared tier references during pool cleanup

Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-09-11 20:23:09 +08:00
parent 51d41345f7
commit ccb676e60c
4 changed files with 160 additions and 7 deletions
+36 -3
View File
@@ -25,6 +25,7 @@ import (
"time"
madmin "github.com/minio/madmin-go/v3"
"github.com/minio/minio/internal/bucket/lifecycle"
xhttp "github.com/minio/minio/internal/http"
)
@@ -232,6 +233,22 @@ func reconcileStoredObjectTags(metadata, stored map[string]string) {
}
}
// A restored version still owns its tier reference even while IsRemote is
// false. Only the last copy of a reference may schedule its contents for GC.
func sharesTierObject(oi ObjectInfo, copies []PoolObjInfo) bool {
ref := oi.TransitionedObject
if ref.Status != lifecycle.TransitionComplete {
return false
}
for _, copy := range copies {
other := copy.ObjInfo.TransitionedObject
if other.Status == lifecycle.TransitionComplete && ref.Tier == other.Tier && ref.Name == other.Name && ref.VersionID == other.VersionID {
return true
}
}
return false
}
// retireReplicaCopies runs only after committing a replacement. Failures are
// returned to the caller, so a stale copy cannot be hidden behind a successful
// response. Data movement owns its source cleanup and does not use this helper.
@@ -244,12 +261,25 @@ func (z *erasureServerPools) retireReplicaCopies(ctx context.Context, bucket, ob
if err != nil {
return err
}
var retained []PoolObjInfo
for _, copy := range copies {
if copy.Index == keep {
retained = []PoolObjInfo{copy}
break
}
}
if len(retained) == 0 {
return VersionNotFound{Bucket: bucket, Object: decodeDirObject(object), VersionID: versionID}
}
for i, copy := range copies {
if copy.Index == keep {
continue
}
_, err := z.serverPools[copy.Index].DeleteObject(ctx, bucket, object,
ObjectOptions{VersionID: versionID, NoLock: true, NoAuditLog: true})
ObjectOptions{
VersionID: versionID, NoLock: true, NoAuditLog: true,
SkipFreeVersion: sharesTierObject(copy.ObjInfo, retained) || sharesTierObject(copy.ObjInfo, copies[i+1:]),
})
if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
return err
}
@@ -304,8 +334,11 @@ func (z *erasureServerPools) deleteObjectConditional(ctx context.Context, bucket
// Retire non-authoritative copies first. If cleanup fails, retain the
// authoritative version and report the error instead of acknowledging a
// deletion that would expose an older copy.
for _, copy := range copies[1:] {
_, err := z.serverPools[copy.Index].DeleteObject(ctx, bucket, object, opts)
for i := 1; i < len(copies); i++ {
candidate := copies[i]
deleteOpts := opts
deleteOpts.SkipFreeVersion = opts.SkipFreeVersion || sharesTierObject(candidate.ObjInfo, copies[:1]) || sharesTierObject(candidate.ObjInfo, copies[i+1:])
_, err := z.serverPools[candidate.Index].DeleteObject(ctx, bucket, object, deleteOpts)
if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
return ObjectInfo{}, err
}
+107
View File
@@ -20,6 +20,7 @@ package cmd
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"maps"
@@ -573,3 +574,109 @@ func TestPoolsReplicaCleanupFailureCanRetry(t *testing.T) {
t.Errorf("retry left the competing version: %v", err)
}
}
func TestPoolsRetiringCopyPreservesSharedTierObject(t *testing.T) {
for _, test := range []struct {
name string
deleting bool
failPrimaryDelete bool
differentRemote bool
restored bool
}{
{name: "metadata-copy"},
{name: "restored-metadata-copy", restored: true},
{name: "metadata-copy-distinct-reference", differentRemote: true},
{name: "failed-primary-delete", deleting: true, failPrimaryDelete: true},
{name: "failed-primary-delete-distinct-reference", deleting: true, failPrimaryDelete: true, differentRemote: true},
{name: "successful-delete", deleting: true},
} {
t.Run(test.name, func(t *testing.T) {
z, bucket := consistencyPools(t)
const object = "shared-tier-object"
metadata := map[string]string{
ReservedMetadataPrefixLower + TransitionStatus: "complete",
ReservedMetadataPrefixLower + TransitionTier: "TEST-TIER",
ReservedMetadataPrefixLower + TransitionedObjectName: "shared-remote-object",
ReservedMetadataPrefixLower + TransitionedVersionID: "shared-remote-version",
}
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})
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,
})
current, err := z.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: oi.VersionID})
if err != nil || current.TransitionedObject.Status != "complete" || current.IsRemote() == test.restored {
t.Fatalf("fixture did not persist the tier reference: %+v, %v", current.TransitionedObject, err)
}
if test.failPrimaryDelete {
// The authoritative copy remains readable if its deletion fails.
// Retiring a secondary copy must not schedule its shared remote
// contents for garbage collection in that case.
set := z.serverPools[0].getHashedSet(object)
getDisks := set.getDisks
faulty := append([]StorageAPI(nil), getDisks()...)
for i := range faulty {
faulty[i] = accessMoveDeleteFaultDisk{StorageAPI: faulty[i], bucket: bucket, object: object, version: oi.VersionID}
}
set.getDisks = func() []StorageAPI { return faulty }
defer func() { set.getDisks = getDisks }()
}
if test.deleting {
_, err := z.DeleteObject(t.Context(), bucket, object, ObjectOptions{
Versioned: true, VersionID: oi.VersionID,
CheckPrecondFn: func(info ObjectInfo) bool { return info.ETag != current.ETag },
})
if (err != nil) != test.failPrimaryDelete {
t.Fatalf("unexpected authoritative delete result: %v", err)
}
} else {
current.metadataOnly = true
current.UserDefined["metadata-update"] = "new"
_, err := z.CopyObject(t.Context(), bucket, object, bucket, object, current,
ObjectOptions{VersionID: oi.VersionID}, ObjectOptions{
Versioned: true, VersionID: oi.VersionID, MTime: oi.ModTime, ReplicaLockReconcile: true,
})
if err != nil {
t.Fatal(err)
}
}
_, err = z.serverPools[0].GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: oi.VersionID})
primaryDeleted := test.deleting && !test.failPrimaryDelete
if primaryDeleted {
if !isErrVersionNotFound(err) {
t.Fatalf("authoritative copy survived successful delete: %v", err)
}
} else if err != nil {
t.Fatalf("lost retained authoritative copy: %v", err)
}
for pool, wantFree := range []bool{primaryDeleted, test.differentRemote} {
for _, disk := range z.serverPools[pool].getHashedSet(object).getDisks() {
data, err := disk.ReadAll(t.Context(), bucket, pathJoin(object, xlStorageFormatFile))
if errors.Is(err, errFileNotFound) && !wantFree {
continue
}
if err != nil {
t.Fatal(err)
}
versions, err := getFileInfoVersions(data, bucket, object, false)
if err != nil {
t.Fatal(err)
}
wantCount := 0
if wantFree {
wantCount = 1
}
if len(versions.FreeVersions) != wantCount {
t.Fatalf("pool %d has %d tier GC markers, want %d", pool, len(versions.FreeVersions), wantCount)
}
}
}
})
}
}
+2 -2
View File
@@ -139,8 +139,8 @@ type ObjectOptions struct {
// when looking up a version by fi.VersionID
InclFreeVersions bool
// SkipFreeVersion skips adding a free version when a tiered version is
// being 'replaced'
// Note: Used only when a tiered object is being expired.
// being replaced. Used when expiring tiered content or retiring a copy
// whose tier reference is still owned by another copy.
SkipFreeVersion bool
MetadataChg bool // is true if it is a metadata update operation.