Repository navigation
fix(ilm): stop taking the free versions of deleted tiered versions for versions - #264
Conversation
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
|
Reviewed and merged into The reported defect is real: a The scope is sufficient and minimal: no storage-format, API, or configuration change is required. The focused regression tests, related migration/listing tests, 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. |
Contribution Licensing (no CLA, inbound=outbound, DCO required)
This pull request contributes to PGSTY SILO (
pgsty/silo). Code contributionsare 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-bytrailer(
git commit -s) certifying theDeveloper 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.ToFileInfoleaves free versions out ofIsLatestandSuccessorModTime. The version lists did not skip it:NoncurrentVersionExpirationrule deleted the latest version;ListObjectVersionsshowed a delete marker on top of a readable object, andListObjectsV2did not list the key;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
getFileInfoVersionswithout free versions recomputesIsLatest,SuccessorModTimeandLatestModTimefrom the remaining versions (cmd/xl-storage-format-utils.go). The scanner is the only caller withinclFreeVersions=falseon main.fix(listing): skip free versions when looking for a latest delete marker
metaCacheEntry.isLatestDeletemarker,xlMetaBuf.IsLatestDeleteMarkerandxlMetaBuf.AllHidden(true)look past free versions for the latest version. A newxlMetaV2.isLatestDeleteMarkerserves 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
metaCacheEntry.fileInfoVersionsreturns the versions without free versions, which go toFreeVersions, as the scanner already gets them. This coversListObjectVersions, the ILM evaluation on listing,Walk, decommission and rebalance. The three healing paths (HealObjects,healErasureSet, the scanner's heal of abandoned objects) use the newallFileInfoVersionsand still see free versions.listPathOptions.shouldSkipskips a key with nothing but free versions when deleted entries are listed, that is, inListObjectVersions. Otherwise such keys take up a page without producing a version, and a page made only of them comes back empty and not truncated.removeMovedVersionshas 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.DELETEhandler usually removes it right away, and the free version only covers a failure.removeMovedVersionsfails.checkAfterDecomcounts 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.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:NoncurrentDays, the latest version is deleted throughdeleteObjectVersions, which does not evaluate the rule again. WithNewerNoncurrentVersions, one more older version than the rule allows is deleted;Upstream minio#19878 made
fileInfoVersionsinclude free versions so that--versionsshows 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, andListObjectVersionsskips keys that have only free versions, so pages do not end early. Free versions stay visible inxl.metaand to the scanner's trace.How to test this PR?
The fixture uses two pools of 16 drives and a versioned bucket in pool 0. The tier
WARMis 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 aNoncurrentDays=1rule. v2 must be the latest version without a successor, and v1 noncurrent since v2.scannerItem.applyActionsmust queue only v1 for expiry.TestFreeVersionListObjects,TestFreeVersionLatestDeleteMarker(commit 2):ListObjectsV2lists 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):ListObjectVersionslistsobj-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.Walkreturns the same versions.TestFreeVersionHealObjects(commit 3):HealObjectsstill 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 withNoncurrentDays=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
TestFreeVersionHealObjectsfails: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], andWalkreturns the same three delete markers;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]andpool 1 holds [DM] of obj-01;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:
getFileInfoVersions;shouldSkipcheck, after which the page-by-page listing returns nothing;removeFreeVersions;checkAfterDecom, after which the decommission with the tier down completes;allFileInfoVersionsinHealObjects.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';go test ./cmd -count=1 -runwithExpireTransitioned,GetFileInfoVersions,PoolsMigration,PoolsDelete,ApplyNewerNoncurrent,ListObjectVersions,ListObjects*,Listing*,MergeXLV2,Heal*,Healing*,DeleteObjects*,DeleteVersion*,Lifecycle*,Tier*,Walk:ok. The full./cmdsuite 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
WARMof typeminio.ListObjectVersionsof a after deleting its latest version v3DM* v2 v1v2* v1ListObjectVersionsof b after deleting its oldest version v1v3* v2 DMv3* v2ListObjectVersionsof c after deleting its only versionDM*ListObjectsV2b da b dGET aafter the decommissionNewerNoncurrentVersions=1v2*, v1 deletedv2* v1With 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.
ListObjectVersionsandWalkno 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.ListObjectsV2already lists the prefix of deleted keys the same way.ListObjectVersionsdecodes the version headers of each key once more, up to the first version that is not free.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
Checklist:
git commit -s) per the DCOcommit-idorPR #here): no, inherited from upstream MinIOmake verifierspassesmake buildpasspgsty/silo.pgsty.com, if needed (not needed)