From 3f13c26eedfc84dcad5ec1ecbd11097944134ce5 Mon Sep 17 00:00:00 2001 From: Denis Babchuk Date: Thu, 29 Oct 2026 22:07:56 +0300 Subject: [PATCH 1/4] fix(ilm): do not let a free version make the latest version noncurrent Signed-off-by: Denis Babchuk --- cmd/erasure-server-pool-free-version_test.go | 256 +++++++++++++++++++ cmd/xl-storage-format-utils.go | 16 ++ 2 files changed, 272 insertions(+) create mode 100644 cmd/erasure-server-pool-free-version_test.go diff --git a/cmd/erasure-server-pool-free-version_test.go b/cmd/erasure-server-pool-free-version_test.go new file mode 100644 index 000000000..345fc7082 --- /dev/null +++ b/cmd/erasure-server-pool-free-version_test.go @@ -0,0 +1,256 @@ +// Copyright (c) 2026 Denis Babchuk +// +// This file is part of Silo Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "context" + "errors" + "slices" + "testing" + "time" + + "github.com/minio/minio/internal/bucket/lifecycle" + objectlock "github.com/minio/minio/internal/bucket/object/lock" +) + +// Deleting a version whose data is in a remote tier leaves a free version in +// its place: a record of the remote object, kept until the scanner removes +// both. A free version is not a version of the object. There is no delete +// marker, and the version below it is the latest one; reads already skip it. + +// freeVersionNoncurrent expires versions one day after they become noncurrent. +const freeVersionNoncurrent = "noncurrentEnabled" + + "1" + +// freeVersionTier is a remote tier held in memory whose removals can fail. +type freeVersionTier struct { + *tierExpiryTestBackend + removeErr error +} + +func (b *freeVersionTier) Remove(ctx context.Context, object string, rv remoteVersionID) error { + if b.removeErr != nil { + return b.removeErr + } + return b.tierExpiryTestBackend.Remove(ctx, object, rv) +} + +type freeVersionTest struct { + t *testing.T + z *erasureServerPools + bucket string + tier *freeVersionTier + labels map[string]string // version ID -> label +} + +// newFreeVersionTest returns two pools with a versioned bucket under rules and +// a remote tier WARM. In pool 0, obj-01 has v1 and v2 and a tiered v3, written +// five, four and three days ago; the client deleted v3. obj-02 had only a +// tiered v1, deleted as well. +func newFreeVersionTest(t *testing.T, rules string) *freeVersionTest { + t.Helper() + z, _ := migrationTestPools(t) + ctx := t.Context() + f := &freeVersionTest{t: t, z: z, bucket: "free-version", labels: map[string]string{}} + if err := z.MakeBucket(ctx, f.bucket, MakeBucketOptions{VersioningEnabled: true}); err != nil { + t.Fatal(err) + } + if rules != "" { + meta, err := globalBucketMetadataSys.Get(f.bucket) + if err != nil { + t.Fatal(err) + } + meta.LifecycleConfigXML = []byte("" + rules + "") + if err := meta.Save(ctx, z); err != nil { + t.Fatal(err) + } + globalBucketMetadataSys.Set(f.bucket, meta) + } + f.tier = &freeVersionTier{tierExpiryTestBackend: &tierExpiryTestBackend{objects: map[string][]byte{}}} + prevTier := globalTierConfigMgr + globalTierConfigMgr = NewTierConfigMgr() + globalTierConfigMgr.drivercache["WARM"] = f.tier + t.Cleanup(func() { globalTierConfigMgr = prevTier }) + + now := UTCNow() + f.put("obj-01", "v1", now.AddDate(0, 0, -5)) + f.put("obj-01", "v2", now.AddDate(0, 0, -4)) + f.deleteVersion(f.tiered("obj-01", "v3", now.AddDate(0, 0, -3))) + f.deleteVersion(f.tiered("obj-02", "v1", now.AddDate(0, 0, -3))) + return f +} + +// put writes a version of object to pool 0. +func (f *freeVersionTest) put(object, label string, mtime time.Time) ObjectInfo { + f.t.Helper() + oi := putConsistencyObject(f.t, f.z, f.bucket, object, 0, object+" "+label, ObjectOptions{Versioned: true, MTime: mtime}) + f.labels[oi.VersionID] = label + return oi +} + +// tiered writes a version of object to pool 0 whose data is in the tier. +func (f *freeVersionTest) tiered(object, label string, mtime time.Time) ObjectInfo { + f.t.Helper() + oi := f.put(object, label, mtime) + infos, errs := readAllFileInfo(f.t.Context(), f.z.serverPools[0].getHashedSet(object).getDisks(), "", f.bucket, object, oi.VersionID, false, false) + if errs[0] != nil { + f.t.Fatal(errs[0]) + } + fi := infos[0] + fi.TransitionStatus, fi.TransitionTier, fi.TransitionedObjName = lifecycle.TransitionComplete, "WARM", f.remote(object, label) + f.tier.objects[fi.TransitionedObjName] = []byte(object + " " + label) + if err := f.z.serverPools[0].DecomTieredObject(f.t.Context(), f.bucket, object, fi, ObjectOptions{}); err != nil { + f.t.Fatal(err) + } + return oi +} + +// remote is the name of the remote object of a tiered version. +func (f *freeVersionTest) remote(object, label string) string { + return f.bucket + "/" + object + "/" + label +} + +// deleteVersion deletes a tiered version by its ID, as a client does, which +// leaves a free version in its place. +func (f *freeVersionTest) deleteVersion(oi ObjectInfo) { + f.t.Helper() + if _, err := f.z.DeleteObject(f.t.Context(), f.bucket, oi.Name, ObjectOptions{Versioned: true, VersionID: oi.VersionID}); err != nil { + f.t.Fatal(err) + } + if got := f.stored(0, oi.Name); len(got) == 0 || got[0] != "free" { + f.t.Fatalf("fixture: %s holds %v, want a free version on top", oi.Name, got) + } +} + +func (f *freeVersionTest) label(fi FileInfo) string { + switch { + case fi.TierFreeVersion(): + return "free" + case fi.Deleted: + return "DM" + case f.labels[fi.VersionID] != "": + return f.labels[fi.VersionID] + } + return "?" +} + +// xlMeta reads the metadata of object in pool from its first drive, nil if +// the pool has none. +func (f *freeVersionTest) xlMeta(pool int, object string) []byte { + f.t.Helper() + disk := f.z.serverPools[pool].getHashedSet(object).getDisks()[0] + buf, err := disk.ReadAll(f.t.Context(), f.bucket, pathJoin(object, xlStorageFormatFile)) + if errors.Is(err, errFileNotFound) { + return nil + } + if err != nil { + f.t.Fatal(err) + } + return buf +} + +// stored returns what pool holds of object, newest first, free versions +// included. +func (f *freeVersionTest) stored(pool int, object string) []string { + f.t.Helper() + buf := f.xlMeta(pool, object) + if buf == nil { + return nil + } + fivs, err := getFileInfoVersions(buf, f.bucket, object, true) + if err != nil { + f.t.Fatal(err) + } + var got []string + for _, fi := range fivs.Versions { + got = append(got, f.label(fi)) + } + return got +} + +// expiryQueue replaces the expiry workers with a queue for the test. +func (f *freeVersionTest) expiryQueue() chan expiryOp { + queue := make(chan expiryOp, 100) + es := &expiryState{ctx: f.t.Context(), objAPI: f.z} + workers := []chan expiryOp{queue} + es.workers.Store(&workers) + prev := globalExpiryState + globalExpiryState = es + f.t.Cleanup(func() { globalExpiryState = prev }) + return queue +} + +// expired returns the labels of the versions queued for expiry. +func (f *freeVersionTest) expired(queue chan expiryOp) []string { + var got []string + for len(queue) > 0 { + switch task := (<-queue).(type) { + case expiryTask: + got = append(got, f.labels[task.objInfo.VersionID]) + case noncurrentVersionsTask: + for _, v := range task.versions { + got = append(got, f.labels[v.VersionID]) + } + } + } + return got +} + +// The scanner reads the versions of obj-01 without the free version. v2 is the +// latest version, and a noncurrent expiry must not count it as noncurrent +// since the deleted v3 was written. Otherwise the scanner deletes it. +func TestFreeVersionScannerLatestVersion(t *testing.T) { + f := newFreeVersionTest(t, freeVersionNoncurrent) + fivs, err := getFileInfoVersions(f.xlMeta(0, "obj-01"), f.bucket, "obj-01", false) + if err != nil { + t.Fatal(err) + } + var got []string + for _, fi := range fivs.Versions { + got = append(got, f.label(fi)) + } + if want := []string{"v2", "v1"}; !slices.Equal(got, want) || len(fivs.FreeVersions) != 1 { + t.Fatalf("scanner versions %v with %d free versions, want %v with 1", got, len(fivs.FreeVersions), want) + } + v2, v1 := fivs.Versions[0], fivs.Versions[1] + if !v2.IsLatest || !v2.SuccessorModTime.IsZero() { + t.Errorf("v2: latest %v, successor mod time %v; want latest without a successor", v2.IsLatest, v2.SuccessorModTime) + } + if v1.IsLatest || !v1.SuccessorModTime.Equal(v2.ModTime) { + t.Errorf("v1: latest %v, successor mod time %v; want noncurrent since v2 at %v", v1.IsLatest, v1.SuccessorModTime, v2.ModTime) + } + if !fivs.LatestModTime.Equal(v2.ModTime) { + t.Errorf("latest mod time %v, want that of v2 %v", fivs.LatestModTime, v2.ModTime) + } + + // Apply the rules to these versions as the scanner does. + lc, err := globalLifecycleSys.Get(f.bucket) + if err != nil { + t.Fatal(err) + } + objInfos := make([]ObjectInfo, len(fivs.Versions)) + for i, fi := range fivs.Versions { + objInfos[i] = fi.ToObjectInfo(f.bucket, "obj-01", true) + } + queue := f.expiryQueue() + item := scannerItem{Path: "obj-01", bucket: f.bucket, objectName: "obj-01", lifeCycle: lc} + item.applyActions(t.Context(), f.z, objInfos, objectlock.Retention{}, &sizeSummary{}, nil) + if expired, want := f.expired(queue), []string{"v1"}; !slices.Equal(expired, want) { + t.Errorf("scanner queued %v for expiry, want %v", expired, want) + } +} diff --git a/cmd/xl-storage-format-utils.go b/cmd/xl-storage-format-utils.go index 13c6836e6..feeefc52b 100644 --- a/cmd/xl-storage-format-utils.go +++ b/cmd/xl-storage-format-utils.go @@ -19,6 +19,7 @@ package cmd import ( "errors" + "time" "github.com/zeebo/xxh3" ) @@ -28,6 +29,8 @@ import ( // - fivs.FreeVersions has all the free versions // // if inclFreeVersions is true all the versions are in fivs.Versions, free and non-free versions alike. +// Otherwise IsLatest, SuccessorModTime and LatestModTime are those of the +// non-free versions. // // Note: Only the scanner requires fivs.Versions to have exclusively non-free versions. This is used while enforcing NewerNoncurrentVersions lifecycle element. func getFileInfoVersions(xlMetaBuf []byte, volume, path string, inclFreeVersions bool) (FileInfoVersions, error) { @@ -56,6 +59,19 @@ func getFileInfoVersions(xlMetaBuf []byte, volume, path string, inclFreeVersions } if !inclFreeVersions { fivs.Versions = fivs.Versions[:n] + // A free version is not a version of the object. The latest + // version, and the successor of each other version, is among + // the rest. + for i := range fivs.Versions { + fivs.Versions[i].IsLatest = i == 0 + fivs.Versions[i].SuccessorModTime = time.Time{} + if i > 0 { + fivs.Versions[i].SuccessorModTime = fivs.Versions[i-1].ModTime + } + } + if n > 0 { + fivs.LatestModTime = fivs.Versions[0].ModTime + } } // Update numversions for i := range fivs.Versions { From fb1a619ee13c6fe6c8469db23ff4d97832b66bc8 Mon Sep 17 00:00:00 2001 From: Denis Babchuk Date: Thu, 29 Oct 2026 22:08:34 +0300 Subject: [PATCH 2/4] fix(listing): skip free versions when looking for a latest delete marker Signed-off-by: Denis Babchuk --- cmd/erasure-server-pool-free-version_test.go | 52 ++++++++++++++++++++ cmd/metacache-entries.go | 10 ++-- cmd/xl-storage-format-v2.go | 34 ++++++++----- 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/cmd/erasure-server-pool-free-version_test.go b/cmd/erasure-server-pool-free-version_test.go index 345fc7082..c4177906a 100644 --- a/cmd/erasure-server-pool-free-version_test.go +++ b/cmd/erasure-server-pool-free-version_test.go @@ -254,3 +254,55 @@ func TestFreeVersionScannerLatestVersion(t *testing.T) { t.Errorf("scanner queued %v for expiry, want %v", expired, want) } } + +// ListObjectsV2 lists obj-01: its latest version is v2, not a delete marker. +// obj-02 has no versions left. +func TestFreeVersionListObjects(t *testing.T) { + f := newFreeVersionTest(t, "") + lo, err := f.z.ListObjectsV2(t.Context(), f.bucket, "", "", "", 100, false, "") + if err != nil { + t.Fatal(err) + } + var got []string + for _, oi := range lo.Objects { + got = append(got, oi.Name) + } + if want := []string{"obj-01"}; !slices.Equal(got, want) { + t.Errorf("ListObjectsV2: %v, want %v", got, want) + } +} + +// The listing checks of the latest version skip the free version: obj-01 is +// shown, obj-02 has nothing to show. Both the encoded metadata and the +// decoded form, which the listing caches, are checked. +func TestFreeVersionLatestDeleteMarker(t *testing.T) { + f := newFreeVersionTest(t, "") + for _, tc := range []struct { + object string + hidden bool + }{{"obj-01", false}, {"obj-02", true}} { + entry := metaCacheEntry{name: tc.object, metadata: f.xlMeta(0, tc.object)} + meta, _, err := isIndexedMetaV2(entry.metadata) + if err != nil || meta == nil { + t.Fatalf("%s: metadata is not indexed: %v", tc.object, err) + } + if got := meta.IsLatestDeleteMarker(); got != tc.hidden { + t.Errorf("%s: IsLatestDeleteMarker %v, want %v", tc.object, got, tc.hidden) + } + if got := meta.AllHidden(true); got != tc.hidden { + t.Errorf("%s: AllHidden(true) %v, want %v", tc.object, got, tc.hidden) + } + if got := meta.AllHidden(false); got != tc.hidden { + t.Errorf("%s: AllHidden(false) %v, want %v", tc.object, got, tc.hidden) + } + if got := entry.isLatestDeletemarker(); got != tc.hidden { + t.Errorf("%s: isLatestDeletemarker %v, want %v", tc.object, got, tc.hidden) + } + if _, err := entry.xlmeta(); err != nil { + t.Fatal(err) + } + if got := entry.isLatestDeletemarker(); got != tc.hidden { + t.Errorf("%s: isLatestDeletemarker of the decoded metadata %v, want %v", tc.object, got, tc.hidden) + } + } +} diff --git a/cmd/metacache-entries.go b/cmd/metacache-entries.go index ee45ac492..a78062d80 100644 --- a/cmd/metacache-entries.go +++ b/cmd/metacache-entries.go @@ -177,14 +177,12 @@ func (e metaCacheEntry) isInDir(dir, separator string) bool { } // isLatestDeletemarker returns whether the latest version is a delete marker. +// Free versions are not versions of the object and are skipped. // If metadata is NOT versioned false will always be returned. // If v2 and UNABLE to load metadata true will be returned. func (e *metaCacheEntry) isLatestDeletemarker() bool { if e.cached != nil { - if len(e.cached.versions) == 0 { - return true - } - return e.cached.versions[0].header.Type == DeleteType + return e.cached.isLatestDeleteMarker() } if !isXL2V1Format(e.metadata) { return false @@ -196,10 +194,10 @@ func (e *metaCacheEntry) isLatestDeletemarker() bool { } // Fall back... xlMeta, err := e.xlmeta() - if err != nil || len(xlMeta.versions) == 0 { + if err != nil { return true } - return xlMeta.versions[0].header.Type == DeleteType + return xlMeta.isLatestDeleteMarker() } // isAllFreeVersions returns if all objects are free versions. diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go index 77248f72a..a014d788a 100644 --- a/cmd/xl-storage-format-v2.go +++ b/cmd/xl-storage-format-v2.go @@ -1138,6 +1138,17 @@ func (x *xlMetaV2) latestModtime() time.Time { return time.Unix(0, x.versions[0].header.ModTime) } +// isLatestDeleteMarker returns true if the latest version that is not a free +// version is a delete marker, or if there is none. +func (x *xlMetaV2) isLatestDeleteMarker() bool { + for _, ver := range x.versions { + if !ver.header.FreeVersion() { + return ver.header.Type == DeleteType + } + } + return true +} + func (x *xlMetaV2) addVersion(ver xlMetaV2Version) error { modTime := ver.getModTime().UnixNano() if !ver.Valid() { @@ -2289,6 +2300,7 @@ func (x xlMetaBuf) ListVersions(volume, path string, allParts bool) ([]FileInfo, } // IsLatestDeleteMarker returns true if latest version is a deletemarker or there are no versions. +// Free versions are not versions of the object and are skipped. // If any error occurs false is returned. func (x xlMetaBuf) IsLatestDeleteMarker() bool { vers, headerV, _, buf, err := decodeXLHeaders(x) @@ -2298,13 +2310,17 @@ func (x xlMetaBuf) IsLatestDeleteMarker() bool { if vers == 0 { return true } - isDeleteMarker := false + isDeleteMarker := true // only free versions _ = decodeVersions(buf, vers, func(idx int, hdr, _ []byte) error { var xl xlMetaV2VersionHeader if _, err := xl.unmarshalV(headerV, hdr); err != nil { + isDeleteMarker = false return errDoneForNow } + if xl.FreeVersion() { + return nil + } isDeleteMarker = xl.Type == DeleteType return errDoneForNow }) @@ -2312,7 +2328,7 @@ func (x xlMetaBuf) IsLatestDeleteMarker() bool { } // AllHidden returns true are no versions that would show up in a listing (ie all free markers) -// Optionally also return early if top is a delete marker. +// Optionally also return early if the top version that is not free is a delete marker. func (x xlMetaBuf) AllHidden(topDeleteMarker bool) bool { vers, headerV, _, buf, err := decodeXLHeaders(x) if err != nil { @@ -2328,16 +2344,12 @@ func (x xlMetaBuf) AllHidden(topDeleteMarker bool) bool { if _, err := xl.unmarshalV(headerV, hdr); err != nil { return errDoneForNow } - if topDeleteMarker && idx == 0 && xl.Type == DeleteType { - hidden = true - return errDoneForNow + if xl.FreeVersion() { + // Check next version + return nil } - if !xl.FreeVersion() { - hidden = false - return errDoneForNow - } - // Check next version - return nil + hidden = topDeleteMarker && xl.Type == DeleteType + return errDoneForNow }) return hidden } From 7ce7fc8d1e206a43bbc7074079efd4abb698d2df Mon Sep 17 00:00:00 2001 From: Denis Babchuk Date: Thu, 29 Oct 2026 22:09:03 +0300 Subject: [PATCH 3/4] fix(pools): do not list or move free versions as delete markers Signed-off-by: Denis Babchuk --- cmd/data-scanner.go | 2 +- cmd/erasure-server-pool-decom.go | 46 +++- cmd/erasure-server-pool-free-version_test.go | 218 +++++++++++++++++++ cmd/erasure-server-pool.go | 2 +- cmd/global-heal.go | 2 +- cmd/metacache-entries.go | 16 +- cmd/metacache-set.go | 5 + cmd/xl-storage-format-utils.go | 4 +- 8 files changed, 288 insertions(+), 7 deletions(-) diff --git a/cmd/data-scanner.go b/cmd/data-scanner.go index 0475c4a53..13d0b99f0 100644 --- a/cmd/data-scanner.go +++ b/cmd/data-scanner.go @@ -776,7 +776,7 @@ func (f *folderScanner) scanFolder(ctx context.Context, folder cachedFolder, int } // We got an entry which we should be able to heal. - fiv, err := entry.fileInfoVersions(bucket) + fiv, err := entry.allFileInfoVersions(bucket) if err != nil { err := bgSeq.queueHealTask(healSource{ bucket: bucket, diff --git a/cmd/erasure-server-pool-decom.go b/cmd/erasure-server-pool-decom.go index 3c21825bb..e0064bfcd 100644 --- a/cmd/erasure-server-pool-decom.go +++ b/cmd/erasure-server-pool-decom.go @@ -982,6 +982,9 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool if decommissioned == len(fivs.Versions) { stopFn := globalDecommissionMetrics.log(decomMetricDecommissionRemoveObject, idx, bi.Name, entry.name) err := z.removeMovedVersions(ctx, bi.Name, entry.name, idx, fivs.Versions) + if err == nil { + err = z.removeFreeVersions(ctx, bi.Name, entry.name, idx, fivs.FreeVersions) + } stopFn(0, err) auditLogDecom(ctx, "DecomDeleteObject", bi.Name, entry.name, "", err) if err != nil { @@ -1108,6 +1111,45 @@ func (z *erasureServerPools) decommissionInBackground(ctx context.Context, idx i return nil } +// removeFreeVersions removes the free versions fvs of object from the pool +// idx being decommissioned, as the scanner removes free versions: first the +// remote object of each, then the free version. A free version is not moved. +// In another pool it would be a delete marker on top of the object, and once +// this pool is gone nothing would remove its remote object. A remote object +// that is already gone is not an error. +func (z *erasureServerPools) removeFreeVersions(ctx context.Context, bucket, object string, idx int, fvs []FileInfo) error { + if len(fvs) == 0 { + return nil + } + for _, fv := range fvs { + err := deleteObjectFromRemoteTier(ctx, fv.TransitionedObjName, fv.TransitionVersionID, fv.TransitionTier) + if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) { + return fmt.Errorf("unable to remove the remote object %s of a free version of %s/%s from tier %s: %w", + fv.TransitionedObjName, bucket, object, fv.TransitionTier, err) + } + } + object = encodeDirObject(object) + lk := z.NewNSLock(bucket, object) + lkctx, err := lk.GetLock(ctx, globalOperationTimeout) + if err != nil { + return err + } + ctx = lkctx.Context() + defer lk.Unlock(lkctx) + for _, fv := range fvs { + _, err := z.serverPools[idx].DeleteObject(ctx, bucket, object, ObjectOptions{ + VersionID: fv.VersionID, + InclFreeVersions: true, + NoLock: true, + NoAuditLog: true, + }) + if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) { + return err + } + } + return nil +} + func (z *erasureServerPools) checkAfterDecom(ctx context.Context, idx int) error { buckets, err := z.getBucketsToDecommission(ctx) if err != nil { @@ -1192,7 +1234,9 @@ func (z *erasureServerPools) checkAfterDecom(ctx context.Context, idx int) error } } - versionsFound += len(fivs.Versions) - ignored + // A free version left behind keeps a remote object that + // nothing removes once the pool is gone. + versionsFound += len(fivs.Versions) - ignored + len(fivs.FreeVersions) }); err != nil { return err } diff --git a/cmd/erasure-server-pool-free-version_test.go b/cmd/erasure-server-pool-free-version_test.go index c4177906a..507e56f3a 100644 --- a/cmd/erasure-server-pool-free-version_test.go +++ b/cmd/erasure-server-pool-free-version_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + madmin "github.com/minio/madmin-go/v3" "github.com/minio/minio/internal/bucket/lifecycle" objectlock "github.com/minio/minio/internal/bucket/object/lock" ) @@ -183,6 +184,29 @@ func (f *freeVersionTest) stored(pool int, object string) []string { return got } +// versions labels the versions of a listing as "