[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

Problem

Decommission and rebalance move each version out of the draining pool through z.PutObject, a multipart upload, z.DeleteObject (delete markers) or z.DecomTieredObject, all with DataMovement set. The pools layer refuses only the source pool as the destination (idx == opts.SrcPoolIdx). Nothing compares the moved version with a copy of the same version ID that another pool already holds. The set layer keeps the moved ModTime and replaces the existing version.

A key can have copies in several pools. An overwrite that lands in another pool leaves the previous null version behind; this became the common case with upstream minio#21115 (83b2ad418b), and #245 stops creating new ones. The full-pool fallback, an interrupted decommission or rebalance, and replication can also leave copies. Reads stay correct because the newest copy wins. A move does not:

1. Key k in an unversioned bucket is written to pool 0 at 10:00 and overwritten at 11:00 into pool 1. Pool 0 keeps the 10:00 null version, and reads return the 11:00 one.
2. Decommission of pool 0 moves the 10:00 version with MTime: 10:00. Pool selection skips pool 0, finds k in pool 1 and writes the old version there, replacing the 11:00 null version.
3. The walker deletes the source copy. Only the 10:00 content is left; the 11:00 write is lost without an error.

On two pools this happens to every such key. With three or more pools, main picks the destination at random; with #245 it is always the pool that holds the current copy. The same window exists without any old duplicate: a client overwrite that commits while a move is copying the previous version is replaced by the moved copy. Unversioned and suspended buckets (null versions) are affected. Versions in a versioned bucket have distinct IDs and are not.

Change

• Check. checkDataMovementCopies (cmd/erasure-server-pool-consistency.go) reads the moved version in every pool with objectPoolInfos. It returns DataMovementOverwriteErr with the new errDataMovementSuperseded when a pool other than the source holds a strictly newer copy.
  • PutObject, CompleteMultipartUpload, the DataMovement branch of DeleteObject and DecomTieredObject call it under the pools-layer object lock they already hold.
  • Multipart moves also check once before copying any part. Completion repeats the check under the lock.
  • For PutObject and completion, the same reads are handed to the set layer's lock and tag reconciliation as replicaObjectInfo, so they replace that read instead of adding one.
  • A DataMovement write without MTime is now rejected. Otherwise the set layer would stamp the moved copy with the current time, and it would win over every other copy. All movers already pass it.
• Walkers. Decommission and rebalance count a superseded version as moved. When all versions of an object are done, the walker removes the source object as usual. The stale duplicate is retired, and checkAfterDecom does not fail on it. Every other DataMovementOverwriteErr is still ignored without counting, as before.
• Tiered versions in decommission. In the tiered branch, failure = err != nil read a shadowed err that was always nil. A failed DecomTieredObject therefore counted as moved, and the source was removed. A failure now counts as a failure and keeps the source, as for other versions. This matters more now, because the check can fail when a pool is unreadable.
• Completion options. Rebalance completion now passes SrcPoolIdx; it was 0 for every source pool. Both movers now pass VersionID to completion, so the check addresses the moved version. The set layer's completion reads neither field.

Equal ModTime is moved as before. The same version ID with the same ModTime is the same write: moves and replication preserve both, and an interrupted move is the usual way to get such a copy. Moving it again also reconciles the copies' Object Lock and tag state (ReplicaLockReconcile). Skipping it would delete the source copy, and with it lock state that deletes currently honour across copies.

Disk reads. Single-part moves make no extra reads. A multipart move makes one extra metadata read per pool, before copying parts. Delete-marker and tiered moves make one per pool; these paths read no other pool before. Ordinary writes are unchanged.

Motivation and Context

The stand is one server with two pools, /data/p1/d{1...4} /data/p2/d{1...4}, on linux/arm64.

• Each run writes 20 keys × 6 times into an unversioned and a versioned bucket.
• It then decommissions pool 1 (mc admin decommission start local/ /data/p1/d{1...4}) or rebalances it. For rebalance the pools sit on tmpfs of different sizes, so that rebalance drains pool 1.
• A key is lost when mc cat does not return its last write.
• "At risk" counts the keys whose newest copy was in pool 2; on main, exactly these keys were lost in every run.

server binary	keys with copies in both pools	decommission: lost (unversioned)	rebalance: lost (unversioned)
pgsty/silo:RELEASE.2026-09-16T00-00-00Z	17–20/20	9, 12, 10, 13 /20	10, 11 /20
main 86fb2b83	17–20/20	10/20	11/20
main + this change	20/20	0/20 (11 at risk)	0/20 (15 at risk)
writes on main, then upgrade to main + #245 + this change	19–20/20	0/20 (8 at risk)	0/20 (11 at risk)

On main, every lost key returns an older write, never a 404. In every run, the versioned bucket loses nothing and lists 6 versions per key. With the change:

• decommission reports Decommission of pool /data/p1/d{1...4} is complete with 0 failed items, and rebalance reports Completed;
• pool 1 holds no xl.meta afterwards, so the stale duplicates are gone as well.

Relation to other changes. #245 removes the main source of new duplicates. This change makes decommission and rebalance safe for the duplicates already on disk, and for writes that race a move.

Refs #207, #245, minio#21115.

How to test this PR?

go test ./cmd -count=1 -v -run '^TestPoolsMigration(KeepsCurrentCopy|YieldsToNewerWrite|ThreePools|RetiresSupersededCopy|TieredFailureKeepsSource)$'

New tests in cmd/erasure-server-pool-migration_test.go use the real multi-pool fixtures and both movers:

• TestPoolsMigrationKeepsCurrentCopy covers decommission and rebalance, single-part and multipart, null and versioned duplicates, source pool 0 and 1, and a source copy that is older, equal or newer. A move never changes what reads return, and an older copy is skipped before any multipart part is copied.
• TestPoolsMigrationYieldsToNewerWrite commits a client overwrite, or a replica of the same version, while the move copies. For multipart, a gated reader makes the check at completion run.
• TestPoolsMigrationThreePools checks that a superseded copy neither replaces the newer copy nor appears in the third pool.
• TestPoolsMigrationRetiresSupersededCopy drives decommissionPool and rebalanceBucket over stale and newer keys: single-part, multipart, versioned, delete marker (suspended bucket) and tiered (decommission only). Reads are unchanged, the source pool is empty, and no version failed.
• TestPoolsMigrationTieredFailureKeepsSource: a tiered move that fails because another pool is unreadable is counted as failed and keeps its source.

Without the change, 36 subtests fail on main and on main + #245, for example stale: reads "old" after the move, want "new". For this check the production code was reverted, and isDataMovementSuperseded was stubbed with isDataMovementOverWriteErr so that the tests build. I also removed each part of the change on its own: the completion check, the early multipart check, SrcPoolIdx or VersionID on completion, the walker accounting for data, marker and tiered versions, and the tiered error handling. Each removal makes at least one test fail. TestPoolsConditionalPutVersionSelection now passes MTime in its data-movement case, as the movers do.

Run locally on Go 1.27.2 (darwin/arm64):

• make verifiers (golangci-lint v2.15.0: 0 issues, check-gen, rebrand-guard), go vet ./cmd, make build
• CGO_ENABLED=0 MINIO_API_REQUESTS_MAX=10000 go test ./cmd/ -count=1 -timeout 30m
• go test -race -count=3 ./cmd -run 'TestPoolsMigration|TestPoolsReplica|TestPoolsConditionalPut|TestPoolsDeleteVersion|Decom|Rebalance'
• go test ./cmd -run 'Pool|Decom|Rebalance|Consistency|Multipart|Placement|Tag' on main + fix(pools): keep overwrites in the pool that holds the object #245
• The stand above.

Compatibility impact

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

• Behaviour. Decommission and rebalance skip a version when another pool already holds a newer copy of it, and retire the stale source copy.
  • Decommission status counts such a version as decommissioned, with its size.
  • Rebalance counts its bytes as freed from the pool, which they are.
• Errors.
  • A skipped version appears in the decommission or rebalance audit entry as data movement skipped, another pool holds a newer copy of …. It is not counted as a failure.
  • A failed tiered move is now counted as failed. Before, it was counted as moved and its source was removed.
• Failure mode. As before for single-part and multipart moves, a move fails when a pool cannot read the version: the set layer already read every pool for DataMovement writes. Delete-marker and tiered moves now fail the same way.
• Rollback restores the previous behaviour. Nothing is migrated. The stale copies that this change retired were not the ones reads returned.

Known issues, not changed here. The first two concern a newer delete marker on the draining pool with a stale copy elsewhere. Both are confirmed with unit tests, on main and with this change.

1. Both walkers skip a single null delete marker ("DELETE marked object with no other non-current versions will be skipped") and remove the source. A stale null version in another pool then becomes visible again.
2. A newer null delete marker moved into a pool that holds an older null version deletes that version, but writes no marker: the set-layer DeleteObject treats it as a versioned delete when the version exists. An older version then becomes visible.
3. When a superseded source version is tiered or restored, the walker removes it with a prefix delete, which schedules no remote cleanup. Its remote object is left in the tier. Nothing still in use is removed.

Happy to open separate issues or a follow-up PR for them.

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): the missing check is inherited. Upstream 83b2ad418b (fix: restrict SinglePool by the minimum free drive threshold minio/minio#21115) made the duplicates it trips on common.
• 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]

Thank you, @botdeniska. Your fix was incorporated in 60c7179dd, preserving your authorship and DCO, and shipped in 20261018000000.

1748f62c7 adds the related delete-marker and source-cleanup safeguards. Final-artifact rebalance/decommission checks passed with 2 and 3 pools, concurrent writes/deletes, and interrupted-operation recovery.

Closing this PR as incorporated through those commits. GitHub will show it as closed rather than merged; the contribution is included and credited in the release. Pool-affinity work remains separate in #245.