Repository navigation
fix(listing): recount mixed-EC null versions when listing all versions - #262
Closed
botdeniska wants to merge 1 commit into
Closed
botdeniska wants to merge 1 commit into
botdeniska wants to merge 1 commit into
Conversation
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Member
|
Reviewed against the current 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 #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 theremergeXLV2Versionscan 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
mergeXLV2Versions(cmd/xl-storage-format-v2.go) no longer requiresrequestedVersions == 1for the mixed-EC null-version recount.singleNullVersionStreams(originalVersions, true)). For such input, the latest version and all versions are the same version, so both listings now select the same copy..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
checkAfterDecomlists 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?
The tests are in
cmd/xl-storage-format-v2-cross-parity_test.goand use the helpers ofcmd/metacache-null-quorum_test.go.TestMergeXLV2LatestNullCrossParityruns every case forrequestedVersions1 and 0. Each EC group must be selected the same way in both modes, in every stream order.TestMergeXLV2LatestNullCrossParityScope: theall-versionscase 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.TestMigrationCrossParityReadableMajoritywrites an object, overwrites it withMaxParity(8+8), and puts the old 12+4xl.metaback on 4 of the 16 drives, as a degraded overwrite leaves it.listObjectsToRebalanceandlistObjectsToDecommissionmust return the key with the newer metadata, andGetObjectNInfoon 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:The two
TestMergeXLV2LatestNullCrossParitycases 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 buildgo test -race ./cmd -count=1 -run '^Test(MergeXLV2LatestNullCrossParity|CountXLV2MixedECGroups|ListCrossParity|MigrationCrossParity)'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./cmdsuite.Manual checks:
xl.metaon 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.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.
WalkwithoutLatestOnly, used by replication resync and batch jobs. Latest-only listings already returned these keys.Types of changes
Checklist:
git commit -s) per the DCOcommit-idorPR #here): no. Upstream behavior, partly fixed bye90cb2067for latest-only listingsmake verifierspassesmake buildpasspgsty/silo.pgsty.com, if needed (not needed)