diff --git a/cmd/erasure-server-pool-delete-all-reasons_test.go b/cmd/erasure-server-pool-delete-all-reasons_test.go index 815ef6868..546f81747 100644 --- a/cmd/erasure-server-pool-delete-all-reasons_test.go +++ b/cmd/erasure-server-pool-delete-all-reasons_test.go @@ -247,6 +247,103 @@ func TestPurgeAllVersionsRefusesCorruptDrive(t *testing.T) { } } +// Half of the drives of an eight-drive set hold metadata that does not +// decode. The key still reads and heals from the other half, but the drives +// cannot tell its history, so the purge refuses the key before it checks the +// drives. It still queues a heal of every version the decodable drives hold, +// the null version by its explicit ID, so that the next purge finds the key +// whole; the drives refuse nothing once healed. +func TestPurgeAllVersionsHealsUndecodableHistory(t *testing.T) { + z := purgeSinglePool(t, 8) + p := newPurgeTest(t, z, "purge-undecodable", MakeBucketOptions{VersioningEnabled: true}, purgeRules) + ctx := t.Context() + const key = "ea/undecodable" + p.put(0, key, "null", daysAgo(7), ObjectOptions{}) + p.version(0, key, "v1", 6) + p.version(0, key, "v2", 5) + ops := []expiryOp{p.deleteAllTask(0, key, lifecycle.DeleteAllVersionsAction)} + + disks := z.serverPools[0].getHashedSet(key).getDisks() + metaPath := pathJoin(key, xlStorageFormatFile) + good, err := disks[0].ReadAll(ctx, p.bucket, metaPath) + if err != nil { + t.Fatal(err) + } + corrupt := slices.Clone(good) + meta, _, err := isIndexedMetaV2(corrupt) + if err != nil { + t.Fatal(err) + } + meta[len(meta)-1] ^= 1 + bad := disks[len(disks)/2:] + for _, disk := range bad { + if err := disk.WriteAll(ctx, p.bucket, metaPath, corrupt); err != nil { + t.Fatal(err) + } + } + if got := p.get(key); got != "v2" { + t.Fatalf("fixture: with half of the drives undecodable the key reads %s, want v2", got) + } + + var healMu sync.Mutex + queued := make(map[string]bool) + hook := func(op PartialOperation) { + if op.Bucket == p.bucket && op.Object == key { + healMu.Lock() + queued[op.VersionID] = true + healMu.Unlock() + } + } + previousHook := mrfPartialOpHook.Swap(&hook) + t.Cleanup(func() { mrfPartialOpHook.Store(previousHook) }) + delta := incompleteDelta(func() { p.run(ops) }) + if delta[ilmIncompleteHistoryUnavailable] != 1 { + t.Errorf("counted %v, want one %s", delta, ilmIncompleteHistoryUnavailable) + } + _, err = z.DeleteObject(ctx, p.bucket, key, ObjectOptions{DeleteAllVersions: true, Expiration: ExpirationOptions{Expire: true}}) + if !isErrWriteQuorum(err) { + t.Errorf("purge on an undecodable history: %v, want a write quorum error", err) + } + if got := p.get(key); got != "v2" { + t.Fatalf("the refused purge left %s readable", got) + } + mrfPartialOpHook.Store(previousHook) + + versions, err := getFileInfoVersions(good, p.bucket, key, false) + if err != nil { + t.Fatal(err) + } + healMu.Lock() + for _, fi := range versions.Versions { + if !queued[deleteAllVersionID(fi.VersionID)] { + t.Errorf("no MRF heal for %q: %v", fi.VersionID, queued) + } + } + if queued[""] { + t.Error("MRF used latest instead of explicit null version") + } + healMu.Unlock() + + for _, fi := range versions.Versions { + id := deleteAllVersionID(fi.VersionID) + if _, err := z.HealObject(ctx, p.bucket, key, id, madmin.HealOpts{}); err != nil { + t.Fatalf("heal %s: %v", id, err) + } + for i, disk := range bad { + if _, err := disk.ReadVersion(ctx, "", p.bucket, key, id, ReadOptions{}); err != nil { + t.Fatalf("version %s on healed drive %d: %v", id, i, err) + } + } + } + if got := p.get(key); got != "v2" { + t.Fatalf("heal changed latest data: %s", got) + } + p.run([]expiryOp{p.deleteAllTask(0, key, lifecycle.DeleteAllVersionsAction)}) + if got := p.held(0, key); got != nil { + t.Errorf("the purge after the heal left %v", got) + } +} + // The scanner of a pool that holds a tiered version of a key whose latest // version is hot does not queue the expiry of all versions, which the purge // would refuse, and counts that it skipped it. It accounts every version of diff --git a/cmd/erasure-server-pool-delete-all.go b/cmd/erasure-server-pool-delete-all.go index f20f8f05c..205f7b816 100644 --- a/cmd/erasure-server-pool-delete-all.go +++ b/cmd/erasure-server-pool-delete-all.go @@ -250,6 +250,21 @@ func (z *erasureServerPools) deleteAllVersions(ctx context.Context, bucket, obje start := time.Now() name := decodeDirObject(object) fail := func(err error) (ObjectInfo, error) { + // A history refused for metadata that did not decode: queue a heal + // of each version the drives show, as deleteAllDrives does for a + // drive it checks, so that a later attempt finds the key whole. The + // purge remains refused. + var undecodable undecodableHistory + if errors.As(err, &undecodable) { + for _, id := range undecodable.ids { + globalMRFState.addPartialOp(PartialOperation{ + Bucket: bucket, + Object: object, + VersionID: id, + Queued: time.Now(), + }) + } + } if isErrReadQuorum(err) { err = InsufficientWriteQuorum{} } @@ -268,6 +283,11 @@ func (z *erasureServerPools) deleteAllVersions(ctx context.Context, bucket, obje all.versions, all.events, all.err = z.lifecycleEventsAllPools(ctx, bucket, object) if all.err != nil || len(all.versions) > 0 { if err := all.expiryApplies(bucket, object, ObjectOptions{DeletePrefix: true}); err != nil { + if all.err != nil { + // The history refused, not the rule: fail with it, which + // callers see as the same write quorum error. + err = all.err + } return fail(err) } } diff --git a/cmd/erasure-server-pool-lifecycle.go b/cmd/erasure-server-pool-lifecycle.go index d3b652b97..af12c7a39 100644 --- a/cmd/erasure-server-pool-lifecycle.go +++ b/cmd/erasure-server-pool-lifecycle.go @@ -29,6 +29,7 @@ import ( "sync" "time" + "github.com/google/uuid" "github.com/minio/minio/internal/bucket/lifecycle" objectlock "github.com/minio/minio/internal/bucket/object/lock" "github.com/minio/minio/internal/bucket/replication" @@ -76,10 +77,11 @@ func (er erasureObjects) objectVersions(ctx context.Context, bucket, object stri majority := len(disks)/2 + 1 var ( - metas [][]xlMetaV2ShallowVersion - metaVs []uint8 - metaV uint8 - unknown int + metas [][]xlMetaV2ShallowVersion + metaVs []uint8 + metaV uint8 + unknown int + undecodable bool ) for i, rf := range rawFileInfos { if errs[i] != nil { @@ -91,6 +93,7 @@ func (er erasureObjects) objectVersions(ctx context.Context, bucket, object stri var xl xlMetaV2 if err := xl.LoadOrConvert(rf.Buf); err != nil { unknown++ + undecodable = true continue } metas = append(metas, xl.versions) @@ -99,7 +102,11 @@ func (er erasureObjects) objectVersions(ctx context.Context, bucket, object stri } // Without an unknown drive the history is complete, also on one drive. if unknown > 0 && unknown >= len(disks)/2 { - return nil, toObjectErr(errErasureReadQuorum, bucket, object) + err := toObjectErr(errErasureReadQuorum, bucket, object) + if undecodable { + return nil, undecodableHistory{ids: shallowVersionIDs(metas), err: err} + } + return nil, err } quorumMetas, quorumErrs := make([]FileInfo, len(disks)), make([]error, len(disks)) @@ -125,6 +132,46 @@ func (er erasureObjects) objectVersions(ctx context.Context, bucket, object stri return versions, nil } +// undecodableHistory is the refusal of objectVersions when too many drives +// could not be read and the metadata of some of them did not decode. It +// wraps the read quorum error and carries the IDs of the versions that the +// drives that decoded hold, nullVersionID for the null version. A drive +// whose metadata does not decode does not come back by itself: a heal +// rewrites it from a quorum. A purge queues one of each version before it +// refuses the key, as deleteAllDrives does for a drive it checks. +type undecodableHistory struct { + ids []string + err error +} + +func (e undecodableHistory) Error() string { + return e.err.Error() + ": the metadata of a drive did not decode" +} + +func (e undecodableHistory) Unwrap() error { return e.err } + +// shallowVersionIDs returns the IDs of the versions in metas that are not +// free versions, each once, nullVersionID for the null version. +func shallowVersionIDs(metas [][]xlMetaV2ShallowVersion) []string { + var ids []string + seen := make(map[[16]byte]bool) + for _, versions := range metas { + for _, ver := range versions { + id := ver.header.VersionID + if ver.header.FreeVersion() || seen[id] { + continue + } + seen[id] = true + if id == [16]byte{} { + ids = append(ids, nullVersionID) + } else { + ids = append(ids, uuid.UUID(id).String()) + } + } + } + return ids +} + // revisionKey identifies a revision of a version: its header and a strong // digest of its metadata. The header Signature is a 32-bit fold of weak // hashes; two revisions can share it. @@ -675,7 +722,7 @@ func (all lifecycleAllPools) protection(versionID string) objectLockState { // or a delete marker without noncurrent versions. func (all lifecycleAllPools) expiryApplies(bucket, object string, opts ObjectOptions) error { if all.err != nil { - if _, ok := all.err.(InsufficientReadQuorum); ok { + if isErrReadQuorum(all.err) { return InsufficientWriteQuorum{} } return all.err