[botdeniska]

Contribution Licensing (no CLA, inbound=outbound, DCO required)

This pull request contributes to PGSTY SILO (pgsty/silo). Code contributions
are accepted under AGPL-3.0-or-later, the same license as the server.
This project does not use a CLA or require a separate Apache-2.0 license grant.
By submitting this pull request I represent that I have the right to contribute
the code changes under this repository's
GNU Affero General Public License v3.0 or later
and retain copyright in my original work. Existing copyright and license
notices remain intact; separately licensed material keeps its applicable terms.
Every commit must carry a DCO Signed-off-by trailer
(git commit -s) certifying the
Developer Certificate of Origin — see
CONTRIBUTING.md.

Description

Fixes #265.

In a versioned bucket, an Expiration that is due on the current version only puts a delete marker on top of it, and the version stays as noncurrent. Decommission and rebalance skip that version as "ILM expired" and never copy it. Decommission then either loses it with the pool or fails, depending on when the queued expiry runs. Rebalance never removes the source copies of that key. In decommission, removeMovedVersions also fails with PreConditionFailed on any version skipped for ILM that the expiry queue has not deleted yet, before it removes the moved versions.

Two commits:

fix(pools): move ILM-expired current versions during decommission

1. filterLifecycle in decommissionPool and checkAfterDecom: DeleteAction on a versioned object (enabled or suspended) is no longer queued and skipped. The version is moved like any other, and the scanner puts the marker on it in the destination pool. checkAfterDecom counts such a version if it is left behind.
2. The filterLifecycle of decommissionPool returns the action it queued. The walker collects the skipped versions that ILM deletes alone: DeleteVersionAction, and DeleteAction on an object without versions (ilmDeletesVersionOnly).
3. removeMovedVersions takes these versions as expired and removes them from the source without requiring a moved copy, as the queued expiry would. The ModTime and delete-marker checks still apply to them. Leaving them to the expiry does not work: once the newer versions are gone from the source pool, a skipped noncurrent version is the latest one there. The expiry re-evaluates the rule against that pool (cmd/erasure-object.go:2154), finds no action and keeps the version, and checkAfterDecom fails the decommission.
4. A version whose action deletes the whole object (DeleteAllVersionsAction, DelMarkerDeleteAllVersionsAction) is not passed. It is the current version. Removed before the expiry runs, it would expose the moved older versions, or bring back a deleted object. It stays in the source until the queued expiry deletes the object in every pool, as on main. The same holds for the expiry of a restored copy (DeleteRestored*), which does not delete the version. Decommission already moves such a version, and rebalance skips it earlier as transitioned.

fix(pools): move ILM-expired current versions during rebalance and remove moved copies

5. filterLifecycle in rebalanceBucket: the same as 1 and 2.
6. The walker counts a version skipped for ILM as handled, as decommission does, and passes it to removeMovedVersions under the same rules as 2 to 4. Before, rebalanced == len(fivs.Versions) never held for such a key, and the moved versions stayed in the source pool.

Motivation and Context

Any object whose Expiration became due since the last scanner pass is exposed during a decommission, and every matching object is exposed when a rule is added or changed shortly before one. The operator sees Complete and loses the version that the rule meant to keep as noncurrent, including a version under COMPLIANCE retention or one in a remote tier. Rebalance loses nothing, but leaves duplicates in the pool it was meant to drain. The scanner does not remove them, and only a later rebalance or decommission walks them again.

How to test this PR?

go test ./cmd -count=1 -v -run '^TestPools(DecommissionLifecycleExpiry|DecommissionCountsExpiredCurrentVersions|RebalanceLifecycleExpiry)$'

The tests are in a new file, cmd/erasure-server-pool-migration-ilm_test.go, and use migrationTestPools and migrationTestWalk. The bucket lives in pool 0 of two pools. The expiry workers are replaced by a queue that nothing drains, so the expiries the walk queues run after it, as when the workers lag behind. A decommission runs as the admin API runs it (StartDecommission, doDecommissionInRoutine, including checkAfterDecom) and must complete. A rebalance runs rebalanceBucket over pool 0. Then ILM catches up: the queued expiries run, and the rules are applied to the current version of every object as the scanner applies them. Pool 0 must be empty, and pool 1 must hold the expected versions.

TestPoolsDecommissionLifecycleExpiry and TestPoolsRebalanceLifecycleExpiry share these cases:

• versioned/expire-current: Expiration.Date in the past, v1 v2 v3; expected DM v3 v2 v1.
• versioned/expire-current-locked: the same with v3 under COMPLIANCE retention; the retention must arrive in pool 1.
• suspended/expire-current: a versioned v3 and a null current version in a suspended bucket; expected DM v3 v2 v1 and DM v1.
• versioned/noncurrent-expiration: NoncurrentDays=1, v1 expired; expected v3 v2, and the source copies removed before the expiry has run.
• versioned/expire-all-versions, versioned/expire-marker-all-versions: ExpiredObjectAllVersions and DelMarkerExpiration. After the move and before the expiry runs, the object must still read as v3, or as deleted. Then all its versions are gone.
• versioned/no-lifecycle, versioned/expired-delete-marker, unversioned/expire-current: unchanged behavior.

Decommission only: versioned/expire-current-tiered (a transitioned v3 is moved as metadata). Rebalance does not move transitioned versions yet. TestPoolsDecommissionCountsExpiredCurrentVersions: checkAfterDecom must count an expired current version left in the pool.

On main dd905b29f, 9 of the 19 subtests and TestPoolsDecommissionCountsExpiredCurrentVersions fail:

decommission expire-current, -locked, suspended:  pool 0 still holds [DM v3]; pool 1 holds [v2 v1], want [DM v3 v2 v1]
decommission expire-current-tiered:              pool 0 still holds [v3]; pool 1 holds [v2 v1], want [v3 v2 v1]
decommission noncurrent-expiration:              decommission did not complete: failed=true; pool 0 holds [v3 v2 v1]
TestPoolsDecommissionCountsExpiredCurrentVersions: decommission check passed with an expired current version left in the pool
rebalance expire-current, -locked, suspended:    pool 0 still holds [DM v3 v2 v1]; pool 1 holds [v2 v1]
rebalance noncurrent-expiration:                 pool 0 still holds [v3 v2]

The other subtests pass on main and guard the unchanged paths. The two "all versions" cases fail when every skipped version is passed to removeMovedVersions: the object reads as v2, or the deleted object reads as v1.

Each of the eight changes was reverted on its own, and each time its subtests failed. The changes are the DeleteAction case in each of the three filterLifecycle closures, the count in rebalance, expired passed in each walker, and the ilmDeletesVersionOnly filter in each walker.

Run locally on Go 1.27.3 (darwin/arm64), on main dd905b29f:

• make verifiers (golangci-lint v2.15.0: 0 issues, check-gen, rebrand-guard), go vet ./cmd, make build
• go test ./cmd -count=1 -run '^(TestPools(Migration|Decommission|Rebalance|Delete|BulkDelete|ConditionalPut|ConditionalDelete)|TestMigration|TestPoolMeta|TestDecom)': ok
• go test ./cmd -count=3 -race -run '^TestPools(DecommissionLifecycleExpiry|DecommissionCountsExpiredCurrentVersions|RebalanceLifecycleExpiry|MigrationCleanupRechecksSource|MigrationDeleteMarkers)$': ok
• go test -tags kqueue,dev,consistency_strict ./cmd -run '^$': ok

Manual check with the script from the issue. The setup is one native server with two pools of 4 directories, and RAM disks of 320 MiB and 1 GiB for rebalance.

	main	this change
decommission, Expiration.Date, 1 key	Complete in 7 of 9 runs, then v3 is gone after p1 is removed; failed in 2	Complete in 9 of 9, v3 v2 v1 in p2, the marker follows
decommission, NoncurrentDays=1, 200 keys, lagging expiry queue	failed, 199 × PreConditionFailed, at least 398 object(s)/version(s); a second run completes	Complete on the first run, v3 v2 in p2
decommission, other rules (5 keys: none, NewerNoncurrentVersions, ExpiredObjectDeleteMarker, unversioned)	Complete; PreConditionFailed logged for lone markers and unversioned keys	Complete, no errors in the log
rebalance, Expiration.Date	p1[DM v3 v2 v1] p2[v2 v1], unchanged after a scanner pass	p1 empty, p2[DM v3 v2 v1]
rebalance, NoncurrentDays=1	p1[v3 v2] p2[v3 v2], unchanged after a scanner pass	p1 empty, p2[v3 v2]

Compatibility impact

No API, configuration, storage metadata, metric, route or dependency changes. Single-pool deployments are unchanged.

• Expired current version of a versioned object. Decommission and rebalance move it, so they move more data than before: the versions they would otherwise lose or leave behind. The delete marker is now added by the scanner in the destination pool, at its next pass, instead of by an expiry queued from the walk. Until then the object reads as it did before the move.
• Versions that ILM deletes (noncurrent expiration, lone markers, objects without versions) are still not moved and still queued for expiry. When the walker removes the moved versions of the key before the expiry has run, it removes these too, as upstream's prefix delete does. The queued expiry then finds nothing, so no lifecycle event or audit entry is sent for that version.
• Versions that take the whole object with them stay in the source pool until the expiry runs, as on main. Rebalance now also removes the moved older versions of such a key from the source, and logs the same PreConditionFailed for the current version that decommission logs on main.
• removeMovedVersions takes a second slice; its only callers are the two walkers and one test.
• Rollback brings back the old behavior. Data moved with the change is the same as data moved by main.

Not changed: rebalance still skips transitioned versions. A lone delete marker is still evaluated against the draining pool alone. A full expiry queue still drops tasks.

Related: #240 changes applyExpiryOnNonTransitionedObjects and expireTransitionedObject so that ILM deletions replicate. The two PRs do not touch the same code. With both, an expired current version gets its marker from the scanner in the destination pool, which #240 covers. A version that removeMovedVersions removes before the queued expiry runs is not removed through those functions, so its purge would not be replicated.

Merge after #264. Without it, a free version that sits above the current version (left when a tiered version is deleted) is taken for the latest delete marker, the current version looks noncurrent, and with a NoncurrentDays rule this change would remove it from the source pool without moving it. Both PRs change the removeMovedVersions call in decommissionPool; the conflict is one line.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Optimization (provides speedup with no functional changes)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• All commits are signed off (git commit -s) per the DCO
• Fixes a regression (If yes, please add commit-id or PR # here): no, inherited from upstream MinIO
• Unit tests added/updated
• make verifiers passes
• Relevant package tests and make build pass
• Compatibility and rollback impact documented
• Internal documentation updated (not needed)
• Public documentation update opened in pgsty/silo.pgsty.com, if needed (not needed)

[Vonng]

I reviewed this against the current main, including #264 and #267. The reported bug is valid, and the focused ILM migration tests pass after forward-porting the change, but this revision is not ready to merge.

1. Replication can be bypassed. For a DeleteVersionAction (and the corresponding version-only path), the walk queues an expiry task and then removeMovedVersions deletes the source version directly with SkipFreeVersion: true. If the queued task runs afterward, it can see VersionNotFound; the normal object-layer expiration and replication-delete path never runs, so a replication target may retain the version.

2. A transitioned noncurrent version can lose its durable tier-cleanup record. Decommission follows the same direct source purge for an expired remote version, while expireTransitionedObject is the code that removes the remote object. If that task is delayed, dropped, or the process restarts, no free version remains to make the remote deletion retryable, and the tier object can be orphaned. Rebalance also deliberately skips transitioned versions; that behavior needs to be made explicit in the final migration contract and tests.

3. The lifecycle decision can become stale. The expired list is computed from the lifecycle configuration loaded during the walk, but it later lets source cleanup bypass the current lifecycle check. The queued expiration re-evaluates the current configuration. If the rule is changed or removed between those steps, the source version can be deleted under the old decision while the queued action no longer qualifies.

Please keep the PR open and revise the ordering and ownership of cleanup so replication and tier deletion are completed or durably recorded before the source metadata is purged. The revised tests should cover replication-enabled version deletion, tier failure/restart with retry, a lifecycle configuration change between scan and cleanup, and the combined #264 regression suite. Rebase the result on current main before requesting another review.

[Vonng]

Thank you for the contribution. The reported lifecycle migration bug is valid, and this change correctly preserves versions that an ILM rule will delete later while moving the remaining data.

I forward-ported the PR onto the current main after #264 and #267, resolved the integration conflict while retaining both original commits, and verified the focused migration tests, go vet ./cmd, and make build. All required CI checks are now green.

I am merging this contribution with its original commits preserved. The remaining integrity work will follow in a maintainer PR: keep replication and tier cleanup durable when migration retires an ILM-selected version, handle lifecycle configuration changes between the walk and cleanup phases, enforce the DeleteAll completion fence, and add the corresponding combined regression tests.

Superseded by the forward-ported merge commit; the remaining findings are tracked for the maintainer follow-up.