From b32a389e9d55b4bc086800f2a58087119c28bfee Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 6 Nov 2026 19:09:54 +0800 Subject: [PATCH] perf(pools): read a key's history once in a migration walk's prechecks A decommission or rebalance walk prechecks each due version of a key with the versions of the key in all pools, to decide whether the version moves or is left to its ILM expiry. Each precheck read and merged the key's whole history from every pool, so a key with V due versions read it V times in the walk alone; with long histories this grew roughly quadratically. Read the history once per key into a cache that the walk keeps for the entry, and evaluate every version's precheck against it. The precheck only chooses between moving a version and leaving it to the expiry; the expiry reads the history again under the object lock before it deletes anything, so a stale cache can only move a version or defer it to a recheck. A read error is kept, so every due version of that key moves. The expiry's own per-version reads are unchanged. Test: decommission and rebalance of a key with 63 due noncurrent versions read its history once, with the same outcome; without the cache they read it 63 times. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Feng Ruohang --- cmd/erasure-server-pool-decom.go | 8 ++- cmd/erasure-server-pool-lifecycle.go | 33 +++++++++- ...ure-server-pool-migration-precheck_test.go | 61 +++++++++++++++++++ cmd/erasure-server-pool-rebalance.go | 8 ++- 4 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 cmd/erasure-server-pool-migration-precheck_test.go diff --git a/cmd/erasure-server-pool-decom.go b/cmd/erasure-server-pool-decom.go index 19e6a01e8..545602f32 100644 --- a/cmd/erasure-server-pool-decom.go +++ b/cmd/erasure-server-pool-decom.go @@ -799,7 +799,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool for setIdx, set := range pool.sets { // filterLifecycle returns an ILM deletion that migration must handle // before retiring the source copy, or NoneAction. - filterLifecycle := func(bucket, object string, fi FileInfo) lifecycle.Action { + filterLifecycle := func(bucket, object string, fi FileInfo, history *migrationHistory) lifecycle.Action { if lc == nil { return lifecycle.NoneAction } @@ -819,7 +819,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool // a noncurrent version and has to be moved like any other. The // scanner applies the expiry once it lives in another pool. return lifecycle.NoneAction - case evt.Action.Delete() && !z.migrationExpiryApplies(ctx, objInfo, evt): + case evt.Action.Delete() && !z.migrationExpiryApplies(ctx, objInfo, evt, history): // Another pool holds versions with which the rule does not // apply. The version moves like any other. return lifecycle.NoneAction @@ -848,10 +848,12 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool var decommissioned int var expired []migrationExpiredVersion + // The key's history in all pools, read once for its versions. + var history migrationHistory for _, version := range fivs.Versions { stopFn := globalDecommissionMetrics.log(decomMetricDecommissionObject, idx, bi.Name, version.Name, version.VersionID) // Apply lifecycle rules on the objects that are expired. - if action := filterLifecycle(bi.Name, version.Name, version); action.Delete() { + if action := filterLifecycle(bi.Name, version.Name, version, &history); action.Delete() { decommissioned++ if ilmDeletesVersionOnly(action) || (action.DeleteAll() && !version.IsRemote()) { // Run the logical expiry before retiring the source. diff --git a/cmd/erasure-server-pool-lifecycle.go b/cmd/erasure-server-pool-lifecycle.go index 597cdf249..ab76c53c2 100644 --- a/cmd/erasure-server-pool-lifecycle.go +++ b/cmd/erasure-server-pool-lifecycle.go @@ -762,7 +762,13 @@ func recheckReadExpiry(ctx context.Context, objectAPI ObjectLayer, oi ObjectInfo // of the object, and a delete marker alone in that pool can hide versions in // another one. Such a version is moved like any other, and the scanner // applies the rule in the pool that holds the object then. -func (z *erasureServerPools) migrationExpiryApplies(ctx context.Context, oi ObjectInfo, evt lifecycle.Event) bool { +// +// The walk reads the history of a key once, into h, for the prechecks of all +// its versions: this decides only whether a version moves or is left to the +// expiry, which reads the history again under the object lock before it +// deletes anything. A read error is kept, so every due version of that key +// moves. +func (z *erasureServerPools) migrationExpiryApplies(ctx context.Context, oi ObjectInfo, evt lifecycle.Event, h *migrationHistory) bool { // The options the expiry deletes with, see // applyExpiryOnNonTransitionedObjects and expireTransitionedObject. opts := ObjectOptions{ @@ -780,6 +786,27 @@ func (z *erasureServerPools) migrationExpiryApplies(ctx context.Context, oi Obje if !z.expiryNeedsAllPools(opts) { return true } - _, err := z.checkExpiryAllPools(ctx, oi.Bucket, encodeDirObject(oi.Name), opts) - return err == nil + object := encodeDirObject(oi.Name) + if h == nil { + h = &migrationHistory{} + } + if !h.loaded { + h.all.versions, h.all.events, h.all.err = z.lifecycleEventsAllPools(ctx, oi.Bucket, object) + h.loaded = true + if f := migrationHistoryLoaded; f != nil { + f(oi.Name) + } + } + return h.all.expiryApplies(oi.Bucket, object, opts) == nil +} + +// migrationHistoryLoaded, when a test sets it, is told each time a migration +// walk reads the history of a key. +var migrationHistoryLoaded func(object string) + +// migrationHistory is the history of one key in all pools, read once by a +// decommission or rebalance walk for the prechecks of its versions. +type migrationHistory struct { + loaded bool + all lifecycleAllPools } diff --git a/cmd/erasure-server-pool-migration-precheck_test.go b/cmd/erasure-server-pool-migration-precheck_test.go new file mode 100644 index 000000000..8c508c74f --- /dev/null +++ b/cmd/erasure-server-pool-migration-precheck_test.go @@ -0,0 +1,61 @@ +// Copyright (c) 2026 PGSTY +// +// 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 ( + "fmt" + "sync" + "testing" + "time" + + "github.com/minio/minio/internal/bucket/versioning" +) + +// The walk of a decommission or a rebalance prechecks every due version of a +// key with the versions of the key in all pools. It reads that history once +// per key, not once per version. +func TestPoolsMigrationExpiryPrecheckReadsHistoryOnce(t *testing.T) { + for _, kind := range []string{"decommission", "rebalance"} { + t.Run(kind, func(t *testing.T) { + m := newMigrationILM(t, versioning.Enabled, false, + migrationILMRule("1")) + const key, versions = "obj-many", 64 + for i := range versions { + m.put(key, fmt.Sprintf("v%02d", i), ObjectOptions{Versioned: true, MTime: UTCNow().AddDate(0, 0, -10).Add(-time.Duration(versions-i) * time.Minute)}) + } + var mu sync.Mutex + loads := map[string]int{} + migrationHistoryLoaded = func(object string) { + mu.Lock() + loads[object]++ + mu.Unlock() + } + t.Cleanup(func() { migrationHistoryLoaded = nil }) + + m.migrate(kind) + + mu.Lock() + n := loads[key] + mu.Unlock() + if n != 1 { + t.Errorf("the walk read the history of %s %d times for %d due versions, want once", key, n, versions-1) + } + m.check(map[string][]string{key: {fmt.Sprintf("v%02d", versions-1)}}) + }) + } +} diff --git a/cmd/erasure-server-pool-rebalance.go b/cmd/erasure-server-pool-rebalance.go index da1dd6a8e..49e10e584 100644 --- a/cmd/erasure-server-pool-rebalance.go +++ b/cmd/erasure-server-pool-rebalance.go @@ -590,7 +590,7 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string, for setIdx, set := range pool.sets { // filterLifecycle returns an ILM deletion that migration must handle // before retiring the source copy, or NoneAction. - filterLifecycle := func(bucket, object string, fi FileInfo) lifecycle.Action { + filterLifecycle := func(bucket, object string, fi FileInfo, history *migrationHistory) lifecycle.Action { if lc == nil { return lifecycle.NoneAction } @@ -605,7 +605,7 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string, // a noncurrent version and has to be moved like any other. The // scanner applies the expiry once it lives in another pool. return lifecycle.NoneAction - case evt.Action.Delete() && !z.migrationExpiryApplies(ctx, objInfo, evt): + case evt.Action.Delete() && !z.migrationExpiryApplies(ctx, objInfo, evt, history): // Another pool holds versions with which the rule does not // apply. The version moves like any other. return lifecycle.NoneAction @@ -638,6 +638,8 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string, var rebalanced int var expired []migrationExpiredVersion + // The key's history in all pools, read once for its versions. + var history migrationHistory for _, version := range fivs.Versions { stopFn := globalRebalanceMetrics.log(rebalanceMetricRebalanceObject, poolIdx, bucket, version.Name, version.VersionID) @@ -648,7 +650,7 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string, } // Apply lifecycle rules on the objects that are expired. - if action := filterLifecycle(bucket, version.Name, version); action.Delete() { + if action := filterLifecycle(bucket, version.Name, version, &history); action.Delete() { // Count the version, or the source copies of the moved // ones stay behind as duplicates. Its own source copy // goes with them when ILM deletes it alone; any other