Skip to content

fix(decom): repeat the walk of a bucket while it removes source versions - #335

Merged
Vonng merged 2 commits into
pgsty:mainfrom
botdeniska:fix/decom-keys-below-a-key
Nov 10, 2026
Merged

Vonng merged 2 commits into
pgsty:mainfrom
botdeniska:fix/decom-keys-below-a-key

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

A key below another key (k/b under k) is a directory inside the directory of that key, and the walk does not descend into the directory of an object. A decommission therefore saw k/b only after k had left the pool. By then the walk was over, so the check after the bucket found k/b and failed the decommission. Each level of keys below a key took one more run.

Two commits:

test(decom): cover keys below a key that the walk does not see

cmd/erasure-server-pool-decom-keys-below_test.go, TestDecommissionMovesKeysBelowAKey: two pools, a versioned bucket, k, k/b and k/b/c in pool 0. One decommission of pool 0 must complete, leave none of the keys in pool 0, and read each one with its content.

fix(decom): repeat the walk of a bucket while it removes source versions

  1. cmd/erasure-server-pool-consistency.go, removeMovedVersions also returns how many source versions it deleted. A version that was already gone, or that is left to the expiry path, is not counted.
  2. cmd/erasure-server-pool-decom.go: the walk becomes decommissionPoolWalk, which returns the number of keys whose source versions it removed. decommissionPool repeats it while that number is above zero, the walk returns no error and the context is not canceled.
  3. cmd/erasure-server-pool-rebalance.go and three tests ignore the new count. Rebalance is unchanged: a key it does not see stays in its pool, which is not removed.

Motivation and Context

The details are in #334. Found while porting the fix for #297 to MinIO RELEASE.2025-07-23T15-54-02Z. There the source copy is removed with a prefix delete, which loses the keys below the moved key. Our port repeats the walk the same way.

How to test this PR?

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

On main, with the test file of this PR:

decommission: complete false, failed true; want complete
k/b still in pool 0: <nil>
k/b/c still in pool 0: <nil>
--- FAIL: TestDecommissionMovesKeysBelowAKey

With the fix it passes.

repro.sh from #334 (one local server, two pools of 4 drives):

On main d13899698:

written: 1 keys listed of 3 (k/b and k/b/c sit below k)
run 1: Failed; k=k k/b=k/b k/b/c=k/b/c
run 2: Failed; k=k k/b=k/b k/b/c=k/b/c
run 3: Complete; k=k k/b=k/b k/b/c=k/b/c
   2 at least 1 object(s)/version(s) were found in bucket `keys-below` after decommissioning
drives and server log: <W>

With this PR:

written: 1 keys listed of 3 (k/b and k/b/c sit below k)
run 1: Complete; k=k k/b=k/b k/b/c=k/b/c
drives and server log: <W>

Run locally on Go 1.27.3 (darwin/arm64). The new test and repro.sh ran on main d13899698. The checks below ran on main 8f3657dc5, which has the same Go code: d13899698 adds only documentation.

  • make verifiers (golangci-lint: 0 issues, check-gen, rebrand checks), gofmt, go vet ./cmd, make build
  • go test ./cmd/ -timeout 40m (534 s): ok; the consistency_strict compile; the CI race sets ^Test(PoolsLifecycle|PoolsMigrationLifecycle|ExpireRestored|NativeRestore) and ^Test(Pools(Delete|BulkDelete|RetiringCopyPreservesSharedTierObject)|BulkDeleteQuorumEvidence): ok
  • go test -race ./cmd -run '^Test(DecommissionMovesKeysBelowAKey|PoolsDecommission|Decommission|PoolMetaDecommission|PoolsMigration)' -count=3: ok
  • repro.sh above on binaries of main and of this branch (CGO_ENABLED=0 go build -tags kqueue -trimpath). After the run with this PR, no xl.meta of the bucket is left on the drives of the first pool.

Compatibility impact

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

  • One more listing per bucket. A pass that removed anything is followed by another listing of the draining pool. After a bucket is emptied, that listing finds nothing. A pass that only finds keys it cannot move removes nothing, and so it does not repeat.
  • The repeats end. Each repeat needs a version deleted from the draining pool, and that pool takes no new writes.
  • Rollback brings back the old behavior: a restart of the decommission per level of keys below a key.

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 walk is the same in MinIO RELEASE.2025-07-23T15-54-02Z
  • Unit tests added/updated
  • make verifiers passes
  • go test ./cmd, the decommission, migration and lifecycle race sets 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>
@Vonng
Vonng merged commit f3b42b0 into pgsty:main Nov 10, 2026
8 checks passed
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.

Decommission fails when a key has keys below it, and each restart moves only one more level

2 participants