Skip to content

fix(listing): recount mixed-EC null versions when listing all versions - #262

Closed
botdeniska wants to merge 1 commit into
pgsty:mainfrom
botdeniska:fix/decom-mixed-ec-null
Closed

botdeniska wants to merge 1 commit into
pgsty:mainfrom
botdeniska:fix/decom-mixed-ec-null

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

Overwriting an object without versions while drives are offline raises parity. The online drives get the new copy, for example 10+6 on 14 of 16 drives, and the drives that were offline keep the old 12+4 copy. GET reads the object. Since e90cb2067, a latest-only listing counts the two EC groups separately, so ListObjectsV2 returns it. Decommission and rebalance list all versions, and there mergeXLV2Versions can still drop the key, depending on the drive order. Decommission skips the object and reports Complete, and the object is gone once the pool is removed. Rebalance leaves the object in the source pool. ListObjectVersions on a bucket without versioning or with versioning suspended does not list it.

One commit:

fix(listing): recount mixed-EC null versions when listing all versions

  1. mergeXLV2Versions (cmd/xl-storage-format-v2.go) no longer requires requestedVersions == 1 for the mixed-EC null-version recount.
    • The recount still runs only when every non-empty stream holds exactly one ordinary null version with an EC header (singleNullVersionStreams(originalVersions, true)). For such input, the latest version and all versions are the same version, so both listings now select the same copy.
    • Strict resolution, version IDs, delete markers, free versions, histories and legacy or partial EC headers stay outside the recount.
    • EC groups are still counted separately. Groups below quorum do not add up to a key.
  2. Tests, below. The new test is added to the listing race job in .github/workflows/go.yml.

Motivation and Context

Decommission is how a pool is retired or replaced. Here it reports success and loses data. Nothing warns before the pool is removed: GET, ListObjectsV2 and the decommission status all look normal, and checkAfterDecom lists the pool with the same function, so it finds nothing left. The affected keys are those overwritten while a drive of their set was offline and not healed since. MRF tries such an object once, about a second after the write, and gives up if the drives are still offline. A GET does not queue a heal, because the old copy's metadata is valid and only its EC layout differs.

Whether main drops a key depends on where the drives with the older copy sit in the set's drive order. In the script from the issue, main loses all 20 keys with d15 and d16 offline, and with d7 and d8. With d1 and d2, it moves all of them.

How to test this PR?

go test ./cmd -count=1 -v -run '^(TestMergeXLV2LatestNullCrossParity|TestMergeXLV2LatestNullCrossParityScope|TestMigrationCrossParityReadableMajority)$'

The tests are in cmd/xl-storage-format-v2-cross-parity_test.go and use the helpers of cmd/metacache-null-quorum_test.go.

  • TestMergeXLV2LatestNullCrossParity runs every case for requestedVersions 1 and 0. Each EC group must be selected the same way in both modes, in every stream order.
  • TestMergeXLV2LatestNullCrossParityScope: the all-versions case asserted the old limit and is removed. Every excluded input (strict, version ID, delete marker, free version, history, legacy and partial EC header) is now checked in both modes and must still select nothing.
  • TestMigrationCrossParityReadableMajority writes an object, overwrites it with MaxParity (8+8), and puts the old 12+4 xl.meta back on 4 of the 16 drives, as a degraded overwrite leaves it. listObjectsToRebalance and listObjectsToDecommission must return the key with the newer metadata, and GetObjectNInfo on the set must read the newer data.

On main, with this test file (git checkout <this branch> -- cmd/xl-storage-format-v2-cross-parity_test.go), 6 subtests fail:

xl-storage-format-v2-cross-parity_test.go:89: requested 0, order 1: got [], want group 1
xl-storage-format-v2-cross-parity_test.go:89: requested 0, order 0: got [], want group 0
xl-storage-format-v2-cross-parity_test.go:89: requested 0, order 1: got [], want group 1
xl-storage-format-v2-cross-parity_test.go:89: requested 0, order 1: got [], want group 1
--- FAIL: TestMergeXLV2LatestNullCrossParity (0.00s)
    --- FAIL: TestMergeXLV2LatestNullCrossParity/new-twelve-old-four (0.00s)
    --- FAIL: TestMergeXLV2LatestNullCrossParity/old-twelve-new-four (0.00s)
    --- FAIL: TestMergeXLV2LatestNullCrossParity/new-nine-old-seven (0.00s)
    --- FAIL: TestMergeXLV2LatestNullCrossParity/eight-meets-eight (0.00s)
xl-storage-format-v2-cross-parity_test.go:278: migration listing skipped a readable object: [], <nil>
xl-storage-format-v2-cross-parity_test.go:278: migration listing skipped a readable object: [], <nil>
--- FAIL: TestMigrationCrossParityReadableMajority (0.20s)
    --- FAIL: TestMigrationCrossParityReadableMajority/rebalance (0.00s)
    --- FAIL: TestMigrationCrossParityReadableMajority/decommission (0.00s)

The two TestMergeXLV2LatestNullCrossParity cases that expect no selection, and the scope test, pass on main too. They guard the limits of the recount.

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 -race ./cmd -count=1 -run '^Test(MergeXLV2LatestNullCrossParity|CountXLV2MixedECGroups|ListCrossParity|MigrationCrossParity)'
  • listing, merge, migration, heal and multi-pool tests: go test ./cmd -count=1 -run '^(Test_mergeXLV2Versions.*|TestMergeXLV2.*|TestResolveSingleNullQuorum|Test_metaCacheEntries_.*|TestList.*|TestListing.*|TestSingleNullQuorum.*|TestIAMListing.*|TestMigration.*|TestPools.*|TestCountXLV2.*|TestHealing.*|TestHealObject.*|TestHealCorrectQuorum|TestExpireTransitioned.*|TestStreamMetadataParts.*|TestLocalLockerListing.*|TestDistributedListing.*|TestResync.*Walk.*)$': ok. I did not run the whole ./cmd suite.

Manual checks:

  • The script from the issue: one server, p1 with 16 drives at EC:4 and p2 with 12, two drives of p1 unavailable while 20 keys (4 KiB and 4 MiB) are overwritten. Half of the keys are in a bucket without versioning, half in a bucket with versioning suspended.
  • A second setup writes the old copy on all 16 drives and the new one on N of them, then restores the old xl.meta on the other drives while the server is stopped. There are 96 keys, inline and 1 MiB, in the same two kinds of bucket. Mixed-EC keys either have a new 12+4 copy on 12 drives over an old 14+2 copy, or an old 12+4 copy on 12 drives under a new 14+2 copy on 4. Each layout is tested with the new copies on the first drives and on the last.
main this change
script, ListObjectVersions before decommission 0 of 20 keys 20 of 20
script, decommission status Complete Complete
script, GET after p1 is removed 0 of 20 20 of 20
script, other drives offline (d7, d8) 0 of 20 after removal 20 of 20
second setup, decommission, keys lost after p1 is removed 16 of 96, in each drive order 0
second setup, rebalance, keys left only in the source pool 16 of 96, status Completed 0

The other keys of the second setup are moved on main and with this change. They have one EC layout, with consistent copies or a newer copy on 1, 2 or 4 drives.

Compatibility impact

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

  • Listings of all versions now return keys whose copies hold one null version in two EC layouts, when one layout has quorum. This covers decommission, rebalance and ListObjectVersions on buckets without versioning or with versioning suspended. It also covers Walk without LatestOnly, used by replication resync and batch jobs. Latest-only listings already returned these keys.
  • Heal and the scanner's abandoned-object pass fall back to the first entry when resolution fails, so they reach these keys either way.
  • Cost. The recount runs only after the normal merge fails to reach quorum, and only for single null-version streams, as for latest-only listings now.
  • Rollback brings back the old behavior. Objects already moved by decommission or rebalance are ordinary objects in the target pool.

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. Upstream behavior, partly fixed by e90cb2067 for latest-only listings
  • 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>
@Vonng Vonng closed this Nov 1, 2026

Vonng commented Nov 1, 2026

Copy link
Copy Markdown
Member

Reviewed against the current main. The reported defect is real: all-version migration listings could drop a readable null version when modern EC layouts coexist after a degraded overwrite.

The necessary and minimal fix is to extend the existing mixed-EC recount to the all-version path while retaining its safety gates: ordinary null versions only, modern EC headers, non-strict resolution, and per-EC-group quorum. That fix is already present in and covered by maintainer PR #267.

This PR is therefore superseded by #267. The issue is closed as completed and this duplicate PR is closed without merging.

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 skips an object without versions whose copies have two EC layouts, reports Complete, and removing the pool loses it

2 participants