Skip to content

fix(ilm): stop taking the free versions of deleted tiered versions for versions - #264

Merged
Vonng merged 3 commits into
pgsty:mainfrom
botdeniska:fix/free-versions-not-versions
Nov 1, 2026
Merged

Vonng merged 3 commits into
pgsty:mainfrom
botdeniska:fix/free-versions-not-versions

Conversation

@botdeniska

Copy link
Copy Markdown

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 #263.

Deleting a version whose data is in a remote tier leaves a free version in xl.meta. It records the remote object until the scanner removes both. Reads skip it: xlMetaV2.ToFileInfo leaves free versions out of IsLatest and SuccessorModTime. The version lists did not skip it:

  • the scanner's lifecycle evaluation took the version below for noncurrent, and a NoncurrentVersionExpiration rule deleted the latest version;
  • ListObjectVersions showed a delete marker on top of a readable object, and ListObjectsV2 did not list the key;
  • decommission and rebalance moved the free version as a plain delete marker, and after a decommission the object read 404.

Three commits, each with its tests in the new cmd/erasure-server-pool-free-version_test.go:

fix(ilm): do not let a free version make the latest version noncurrent

  1. getFileInfoVersions without free versions recomputes IsLatest, SuccessorModTime and LatestModTime from the remaining versions (cmd/xl-storage-format-utils.go). The scanner is the only caller with inclFreeVersions=false on main.

fix(listing): skip free versions when looking for a latest delete marker

  1. metaCacheEntry.isLatestDeletemarker, xlMetaBuf.IsLatestDeleteMarker and xlMetaBuf.AllHidden(true) look past free versions for the latest version. A new xlMetaV2.isLatestDeleteMarker serves the decoded form. A key whose only entries are free versions is still a deleted key, as before.

fix(pools): do not list or move free versions as delete markers

  1. metaCacheEntry.fileInfoVersions returns the versions without free versions, which go to FreeVersions, as the scanner already gets them. This covers ListObjectVersions, the ILM evaluation on listing, Walk, decommission and rebalance. The three healing paths (HealObjects, healErasureSet, the scanner's heal of abandoned objects) use the new allFileInfoVersions and still see free versions.
  2. listPathOptions.shouldSkip skips a key with nothing but free versions when deleted entries are listed, that is, in ListObjectVersions. Otherwise such keys take up a page without producing a version, and a page made only of them comes back empty and not truncated.
  3. Decommission does not move free versions. After removeMovedVersions has removed the moved versions of a key, removeFreeVersions (cmd/erasure-server-pool-decom.go) does what the scanner's tier sweep does: it removes the remote object of each free version, then the free version, under the pools-layer object lock.
    • A remote object that is already gone is not an error. The DELETE handler usually removes it right away, and the free version only covers a failure.
    • When the tier fails, the error is logged and the free version stays in the pool, as when removeMovedVersions fails.
    • checkAfterDecom counts free versions as it did while they were listed as versions, so such a decommission ends as failed. A retry finishes it once the tier is back.
  4. Rebalance only stops moving free versions. The source pool stays in the cluster, and its scanner removes them as usual.

Motivation and Context

Every client DELETE ?versionId= of a tiered version leaves a free version, and so do an overwrite of a tiered null version and an ILM expiry whose remote delete failed. Until the scanner removes it:

  • the scanner pass that removes it first evaluates the lifecycle rules with it present. With NoncurrentDays, the latest version is deleted through deleteObjectVersions, which does not evaluate the rule again. With NewerNoncurrentVersions, one more older version than the rule allows is deleted;
  • a decommission turns it into a delete marker that the scanner never removes. Where the deleted version was the latest one, the object reads 404 after the decommission, and its remote object is no longer referenced.

Upstream minio#19878 made fileInfoVersions include free versions so that --versions shows every entry for supportability. The free versions' author objected there that they are internal and should not be visible through the S3 API. This change keeps them out of S3 listings and data movement. Healing still sees them, and ListObjectVersions skips keys that have only free versions, so pages do not end early. Free versions stay visible in xl.meta and to the scanner's trace.

How to test this PR?

go test ./cmd -count=1 -v -run '^TestFreeVersion'

The fixture uses two pools of 16 drives and a versioned bucket in pool 0. The tier WARM is held in memory, and its removals can be made to fail. obj-01 has v1 and v2, written 5 and 4 days ago, and a tiered v3 from 3 days ago that the client deleted. obj-02 had only a tiered v1, also deleted.

  • TestFreeVersionScannerLatestVersion (commit 1): reads obj-01 as the scanner does, with a NoncurrentDays=1 rule. v2 must be the latest version without a successor, and v1 noncurrent since v2. scannerItem.applyActions must queue only v1 for expiry.
  • TestFreeVersionListObjects, TestFreeVersionLatestDeleteMarker (commit 2): ListObjectsV2 lists obj-01. The latest-marker checks on the encoded and decoded metadata see obj-01 as present and obj-02 as deleted.
  • TestFreeVersionListObjectVersions (commit 3): ListObjectVersions lists obj-01 v2 latest, obj-01 v1, also page by page with one key a page, with and without a delimiter, and with a key of only a free version listed first. Walk returns the same versions.
  • TestFreeVersionHealObjects (commit 3): HealObjects still visits the free versions. It passes on main and guards the healing paths.
  • TestFreeVersionDataMovement (commit 3): a decommission and a rebalance walk of pool 0, without lifecycle rules and with NoncurrentDays=1. Pool 1 must hold v2 on top and no delete marker, and obj-01 must read v2. No walk may queue v2 for expiry. After a decommission, pool 0 holds nothing, and the remote objects of both deleted versions are removed. After a rebalance, the free versions stay in pool 0 for its scanner.
  • TestFreeVersionDecommissionTierUnavailable (commit 3): a full decommission (StartDecommission, doDecommissionInRoutine) while the tier fails removals. It must end as failed, with v2 and v1 moved, the free versions left in pool 0 and obj-01 readable. A retry with the tier back must complete and remove both remote objects.

On main, every new test except TestFreeVersionHealObjects fails:

  • scanner: v2: latest false, successor mod time <v3>, scanner queued [v2 v1] for expiry, want [v1];
  • ListObjectsV2: [], want [obj-01];
  • ListObjectVersions: [obj-00 DM latest obj-01 DM latest obj-01 v2 obj-01 v1 obj-02 DM latest], and Walk returns the same three delete markers;
  • decommission and rebalance: pool 1 holds [DM v2 v1] of obj-01, obj-01 reads "<Object not found: free-version/obj-01>"; with the rule, decommission queued the latest version v2 for expiry: [v1 v2] and pool 1 holds [DM] of obj-01;
  • tier unavailable: decommission with the tier unavailable: complete true, failed false; want failed.

The new tests of each commit, except the healing guard, also fail on the commit before it. Each part of the change was reverted on its own, and each time a test failed:

  • the recomputation in getFileInfoVersions;
  • the latest-marker checks;
  • the shouldSkip check, after which the page-by-page listing returns nothing;
  • removeFreeVersions;
  • the free versions in checkAfterDecom, after which the decommission with the tier down completes;
  • allFileInfoVersions in HealObjects.

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=3 -race -run '^TestFreeVersion';
  • the existing tests around the changed code: go test ./cmd -count=1 -run with ExpireTransitioned, GetFileInfoVersions, PoolsMigration, PoolsDelete, ApplyNewerNoncurrent, ListObjectVersions, ListObjects*, Listing*, MergeXLV2, Heal*, Healing*, DeleteObjects*, DeleteVersion*, Lifecycle*, Tier*, Walk: ok. The full ./cmd suite was not run.

Manual check with the script from the issue: two native servers, hot with p1 and then p1+p2 of 4 drives, and warm as tier WARM of type minio.

main this change
ListObjectVersions of a after deleting its latest version v3 DM* v2 v1 v2* v1
ListObjectVersions of b after deleting its oldest version v1 v3* v2 DM v3* v2
ListObjectVersions of c after deleting its only version DM* nothing
ListObjectsV2 b d a b d
decommission of p1 complete complete
GET a after the decommission 404 v2
n/e (v1 v2, v3 deleted) after the scanner, NewerNoncurrentVersions=1 v2*, v1 deleted v2* v1

With the change, a separate run stopped warm before the decommission. The decommission failed, and the log showed unable to remove the remote object ... of a free version of bkt/a from tier WARM: ... connection refused. The listings stayed correct. After warm was back, a second decommission completed with the same final state.

Compatibility impact

No API, configuration, storage format, metric, route or dependency changes.

  • ListObjectVersions and Walk no longer return free versions as delete markers, as before fix: make sure we list freeVersions like DEL marker with --versions minio/minio#19878. With a delimiter, a prefix that holds only such keys is still listed as a common prefix. ListObjectsV2 already lists the prefix of deleted keys the same way. ListObjectVersions decodes the version headers of each key once more, up to the first version that is not free.
  • Lifecycle rules count noncurrent days from the next real version. A version that was taken for noncurrent too early now expires at its actual date.
  • Decommission calls the tier once for each free version in the pool. Usually the remote object is already gone. While the tier is unreachable or removed from the configuration, a decommission of a pool with free versions ends as failed instead of turning them into delete markers. The error names the tier and the object.
  • Rebalance leaves free versions in the source pool, where the scanner removes them.
  • Healing is unchanged.
  • Rollback brings back the old behavior. Free versions that this change removed during a decommission are those the scanner would have removed.

Not fixed: delete markers that earlier decommissions or rebalances made from free versions cannot be told apart from client markers. Versions that the scanner already deleted this way cannot be restored.

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)

Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
@Vonng
Vonng merged commit 0cfca29 into pgsty:main Nov 1, 2026
8 checks passed

Vonng commented Nov 1, 2026

Copy link
Copy Markdown
Member

Reviewed and merged into main.

The reported defect is real: a free version is an internal tier-cleanup record, not an S3 object version. Treating it as a normal delete marker corrupted lifecycle decisions, listings, and pool migration. The merged fix keeps free versions out of ordinary listing, lifecycle, walking, decommission, and rebalance views; preserves them for healing and tier cleanup; and fails decommission safely when tier cleanup cannot complete.

The scope is sufficient and minimal: no storage-format, API, or configuration change is required. The focused regression tests, related migration/listing tests, go vet ./cmd, and make build passed on top of the current main, including #267.

One limitation remains by design: delete markers already materialized by older buggy migration cannot be distinguished from client-created markers and are not automatically reconstructed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants