From f225ca331203032888889c1c4a3f96ee3452b9f9 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 4 Feb 2024 14:36:13 -0800 Subject: [PATCH] Add more advanced cases for dangling (#18968) --- cmd/erasure-healing.go | 66 +++++++++++----------- cmd/erasure-healing_test.go | 108 ++++++++++++++++++++++++++---------- 2 files changed, 112 insertions(+), 62 deletions(-) diff --git a/cmd/erasure-healing.go b/cmd/erasure-healing.go index 9deb9ab99..79c6f9d97 100644 --- a/cmd/erasure-healing.go +++ b/cmd/erasure-healing.go @@ -900,11 +900,10 @@ func isObjectDangling(metaArr []FileInfo, errs []error, dataErrs []error) (valid // We can consider an object data not reliable // when xl.meta is not found in read quorum disks. // or when xl.meta is not readable in read quorum disks. - danglingErrsCount := func(cerrs []error) (int, int, int) { + danglingErrsCount := func(cerrs []error) (int, int) { var ( notFoundCount int - corruptedCount int - driveNotFoundCount int + nonActionableCount int ) for _, readErr := range cerrs { if readErr == nil { @@ -913,29 +912,16 @@ func isObjectDangling(metaArr []FileInfo, errs []error, dataErrs []error) (valid switch { case errors.Is(readErr, errFileNotFound) || errors.Is(readErr, errFileVersionNotFound): notFoundCount++ - case errors.Is(readErr, errFileCorrupt): - corruptedCount++ default: // All other errors are non-actionable - driveNotFoundCount++ + nonActionableCount++ } } - return notFoundCount, corruptedCount, driveNotFoundCount + return notFoundCount, nonActionableCount } - ndataErrs := make([]error, len(dataErrs)) - for i := range dataErrs { - if errs[i] != dataErrs[i] { - // Only count part errors, if the error is not - // same as xl.meta error. This is to avoid - // double counting when both parts and xl.meta - // are not available. - ndataErrs[i] = dataErrs[i] - } - } - - notFoundMetaErrs, corruptedMetaErrs, driveNotFoundMetaErrs := danglingErrsCount(errs) - notFoundPartsErrs, corruptedPartsErrs, driveNotFoundPartsErrs := danglingErrsCount(ndataErrs) + notFoundMetaErrs, nonActionableMetaErrs := danglingErrsCount(errs) + notFoundPartsErrs, nonActionablePartsErrs := danglingErrsCount(dataErrs) for _, m := range metaArr { if m.IsValid() { @@ -945,10 +931,9 @@ func isObjectDangling(metaArr []FileInfo, errs []error, dataErrs []error) (valid } if !validMeta.IsValid() { - // validMeta is invalid because notFoundPartsErrs is - // greater than parity blocks, thus invalidating the FileInfo{} - // every dataErrs[i], metaArr[i] is an empty FileInfo{} - dataBlocks := (len(ndataErrs) + 1) / 2 + // validMeta is invalid because all xl.meta is missing apparently + // we should figure out if dataDirs are also missing > dataBlocks. + dataBlocks := (len(dataErrs) + 1) / 2 if notFoundPartsErrs > dataBlocks { // Not using parity to ensure that we do not delete // any valid content, if any is recoverable. But if @@ -965,25 +950,40 @@ func isObjectDangling(metaArr []FileInfo, errs []error, dataErrs []error) (valid return validMeta, false } - if driveNotFoundMetaErrs > 0 || driveNotFoundPartsErrs > 0 { + if nonActionableMetaErrs > 0 || nonActionablePartsErrs > 0 { return validMeta, false } if validMeta.Deleted { // notFoundPartsErrs is ignored since // - delete marker does not have any parts - return validMeta, corruptedMetaErrs+notFoundMetaErrs > len(errs)/2 + dataBlocks := (len(errs) + 1) / 2 + return validMeta, notFoundMetaErrs > dataBlocks } - totalErrs := notFoundMetaErrs + corruptedMetaErrs + notFoundPartsErrs + corruptedPartsErrs - if validMeta.IsRemote() { - // notFoundPartsErrs is ignored since - // - transition status of complete has no parts - totalErrs = notFoundMetaErrs + corruptedMetaErrs + quorum := validMeta.Erasure.DataBlocks + if validMeta.Erasure.DataBlocks == validMeta.Erasure.ParityBlocks { + quorum++ } - // We have valid meta, now verify if we have enough files with parity blocks. - return validMeta, totalErrs > validMeta.Erasure.ParityBlocks + // TODO: It is possible to replay the object via just single + // xl.meta file, considering quorum number of data-dirs are still + // present on other drives. + // + // However this requires a bit of a rewrite, leave this up for + // future work. + + if notFoundMetaErrs > 0 && notFoundMetaErrs >= quorum { + // All xl.meta is beyond data blocks missing, this is dangling + return validMeta, true + } + + if notFoundPartsErrs > 0 && notFoundPartsErrs >= quorum { + // All data-dir is beyond data blocks missing, this is dangling + return validMeta, true + } + + return validMeta, false } // HealObject - heal the given object, automatically deletes the object if stale/corrupted if `remove` is true. diff --git a/cmd/erasure-healing_test.go b/cmd/erasure-healing_test.go index 5bc13b270..b5c90e7e9 100644 --- a/cmd/erasure-healing_test.go +++ b/cmd/erasure-healing_test.go @@ -41,6 +41,10 @@ func TestIsObjectDangling(t *testing.T) { fi := newFileInfo("test-object", 2, 2) fi.Erasure.Index = 1 + ifi := newFileInfo("test-object", 2, 2) + ifi.SetInlineData() + ifi.Erasure.Index = 1 + testCases := []struct { name string metaArr []FileInfo @@ -129,13 +133,77 @@ func TestIsObjectDangling(t *testing.T) { expectedMeta: FileInfo{}, expectedDangling: false, }, + { + name: "FileInfoUnDecided-case4", + metaArr: []FileInfo{ + {}, + {}, + {}, + ifi, + }, + errs: []error{ + errFileNotFound, + errFileCorrupt, + errFileCorrupt, + nil, + }, + dataErrs: nil, + expectedMeta: ifi, + expectedDangling: false, + }, + { + name: "FileInfoUnDecided-case5-(ignore errFileCorrupt error)", + metaArr: []FileInfo{ + {}, + {}, + {}, + fi, + }, + errs: []error{ + errFileNotFound, + errFileCorrupt, + nil, + nil, + }, + dataErrs: []error{ + errFileCorrupt, + errFileNotFound, + nil, + errFileCorrupt, + }, + expectedMeta: fi, + expectedDangling: false, + }, + { + name: "FileInfoUnDecided-case6-(data-dir intact)", + metaArr: []FileInfo{ + {}, + {}, + {}, + fi, + }, + errs: []error{ + errFileNotFound, + errFileNotFound, + errFileNotFound, + nil, + }, + dataErrs: []error{ + errFileNotFound, + errFileCorrupt, + nil, + nil, + }, + expectedMeta: fi, + expectedDangling: false, + }, { name: "FileInfoDecided-case1", metaArr: []FileInfo{ {}, {}, {}, - fi, + ifi, }, errs: []error{ errFileNotFound, @@ -144,25 +212,7 @@ func TestIsObjectDangling(t *testing.T) { nil, }, dataErrs: nil, - expectedMeta: fi, - expectedDangling: true, - }, - { - name: "FileInfoDecided-case2", - metaArr: []FileInfo{ - {}, - {}, - {}, - fi, - }, - errs: []error{ - errFileNotFound, - errFileCorrupt, - errFileCorrupt, - nil, - }, - dataErrs: nil, - expectedMeta: fi, + expectedMeta: ifi, expectedDangling: true, }, { @@ -175,8 +225,8 @@ func TestIsObjectDangling(t *testing.T) { }, errs: []error{ errFileNotFound, - errFileCorrupt, - errFileCorrupt, + errFileNotFound, + errFileNotFound, nil, }, dataErrs: nil, @@ -184,26 +234,26 @@ func TestIsObjectDangling(t *testing.T) { expectedDangling: true, }, { - name: "FileInfoDecided-case2-(duplicate data errors)", + name: "FileInfoDecided-case3-(enough data-dir missing)", metaArr: []FileInfo{ {}, {}, {}, - {Deleted: true}, + fi, }, errs: []error{ errFileNotFound, - errFileCorrupt, - errFileCorrupt, + errFileNotFound, + nil, nil, }, dataErrs: []error{ errFileNotFound, - errFileCorrupt, - nil, + errFileNotFound, nil, + errFileNotFound, }, - expectedMeta: FileInfo{Deleted: true}, + expectedMeta: fi, expectedDangling: true, }, // Add new cases as seen