Skip to content

fix(ilm): expire a restored copy by the version it belongs to - #282

Open
botdeniska wants to merge 1 commit into
pgsty:mainfrom
botdeniska:fix/restored-copy-expiry
Open

botdeniska wants to merge 1 commit into
pgsty:mainfrom
botdeniska:fix/restored-copy-expiry

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

When the restore of the current version of a transitioned object expires, Lifecycle.Eval returns DeleteRestoredAction, and expireTransitionedObject called DeleteObject for it without a version ID. The delete then applied to the object, not to the restored copy: a versioned bucket got a delete marker that hid the object, and with versioning suspended the delete went to the null version, removing its data if there was one and failing with VersionNotFound if not. DeleteRestoredVersionAction, for noncurrent versions, already passed the version ID and was not affected.

One commit:

fix(ilm): expire a restored copy by the version it belongs to

  1. expireTransitionedObject (cmd/bucket-lifecycle.go) passes the version ID of the restored version in its DeleteRestored() branch for both actions. An empty version ID stands for the null version and is sent as null: the ListObjects max-keys=1 shortcut reads the latest version without the bucket's versioning, so a null version reaches the expiry with an empty ID even in a versioned bucket. With a version ID, DeleteObject takes the path the noncurrent case already takes: the restore headers of that version are dropped and its data directory is removed. Nothing else in the branch changes.
  2. New cmd/bucket-lifecycle-restore_test.go, TestExpireRestoredCopy. It builds the state through the object layer: it writes the versions, transitions one to an in-memory tier with TransitionObject, restores it with RestoreTransitionedObject, and moves the expiry date in x-amz-restore one day into the past. Then it runs the expiry along each path that applies ILM: the scanner (applyActions, then an expiry worker), a HEAD (applyExpiryRule), ListObjects with max-keys=1 plus ListObjectVersions, and the synchronous call of decommission and drive heal (applyExpiryOnTransitionedObject). Seven layouts (versioning enabled, suspended or off; restored current, noncurrent or null version; suspended with and without a null version under the current one), on one pool and on two. It checks that the versions in each pool are unchanged and there are no free versions, that a GET of the object returns the same version, that the restored data directory is gone from every drive, that the version is still transitioned without x-amz-restore, that a GET of it reads the tier, and that every other version still reads its own data.

The test restores through z.serverPools[0]. With more than one pool, erasureServerPools.RestoreTransitionedObject holds the object lock, and the PutObject of the restored copy inside erasureObjects.restoreTransitionedObject waits for that same lock (putRestoreOpts does not set NoLock). That is a separate problem and not addressed here.

Motivation and Context

A restore gives a transitioned object a local copy for a while. Once it expired, the object looked deleted: GET and HEAD returned 404 NoSuchKey, and the delete marker stayed. A plain GET or HEAD after the expiry date was enough, since both queue the expiry and still serve the object once. With versioning suspended, an older null version could lose its data, or the restored copy stayed on the drives for good.

This does not overlap #240: that change leaves the expiry of restored copies local and touches only the branch for transitioned objects below it.

How to test this PR?

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

On main, with this test file (git checkout <this branch> -- cmd/bucket-lifecycle-restore_test.go), 32 of 56 cases fail, for every path and both pool counts: versioning enabled with a restored current version or null version, and versioning suspended with a restored current version, with and without a null version under it. For example:

versions in each pool: before ["v2 v1"], after ["DM v2 v1"] with 0 free versions
GET reads "", Object not found: restore-versioned-scanner/obj; want "v2"
restored copy of v2 still on 4 drives
GET v1: Object not found: restore-suspended-scanner/obj

The cases that pass on main too (a restored noncurrent version, a restored null current version with versioning suspended, an unversioned bucket) guard the paths that already worked. With the fix all 56 pass. Without the null mapping only versioned-null/list fails, on both pool counts.

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

  • make verifiers (golangci-lint v2.15.0: 0 issues, check-gen, rebrand checks)
  • go vet ./cmd, go test ./internal/bucket/lifecycle -count=1
  • lifecycle, tier, restore, scanner and pool tests in ./cmd: go test ./cmd -count=1 -run '^Test(.*Lifecycle.*|.*Restore.*|.*Transition.*|.*Tier.*|.*Scanner.*|DataUsage.*|Apply.*|EvalAction.*|.*ILM.*|.*Expir.*|.*FreeVersion.*|.*DelMarker.*|Pools.*|Migration.*|ObjectIsRemote)$': ok. I did not run the whole ./cmd suite.
  • make build

No live reproduction: the server sets a restore to expire at UTC midnight at least a day ahead, and the API cannot move that date back, so the test moves it instead.

Compatibility impact

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

  • Restored copies of current versions are now removed by themselves when their restore expires. Before, a versioned bucket got a delete marker on the object instead. Markers that this already created stay; they are ordinary delete markers and can be removed with DELETE ?versionId=.
  • Suspended buckets. The expiry no longer touches the null version when the current version has an ID. Restored copies that stayed because the expiry failed are removed on the next pass.
  • Rollback brings back the old behavior.

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. The branch has never passed a version ID for DeleteRestoredAction; the code is the same in MinIO RELEASE.2025-07-23T15-54-02Z and minio/minio master
  • 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expiry of the restored copy of a current version deletes the object instead of the copy

1 participant