From f92450d8b369346a70dfe1e7e82a5bf60f83e595 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Fri, 14 Apr 2023 16:23:28 -0700 Subject: [PATCH] commonParity should pick readable FileInfo (#17032) --- cmd/erasure-healing-common_test.go | 130 +++++++++++++++++++++++++++++ cmd/erasure-healing_test.go | 14 ++++ cmd/erasure-metadata.go | 22 +++-- 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/cmd/erasure-healing-common_test.go b/cmd/erasure-healing-common_test.go index 5f90dffa9..20b0395cd 100644 --- a/cmd/erasure-healing-common_test.go +++ b/cmd/erasure-healing-common_test.go @@ -655,3 +655,133 @@ func TestDisksWithAllParts(t *testing.T) { } } } + +func TestCommonParities(t *testing.T) { + // This test uses two FileInfo values that represent the same object but + // have different parities. They occur in equal number of drives, but only + // one has read quorum. commonParity should pick the parity corresponding to + // the FileInfo which has read quorum. + fi1 := FileInfo{ + Volume: "mybucket", + Name: "myobject", + VersionID: "", + IsLatest: true, + Deleted: false, + ExpireRestored: false, + DataDir: "4a01d9dd-0c5e-4103-88f8-b307c57d212e", + XLV1: false, + ModTime: time.Date(2023, time.March, 15, 11, 18, 4, 989906961, time.UTC), + Size: 329289, Mode: 0x0, WrittenByVersion: 0x63c77756, + Metadata: map[string]string{ + "content-type": "application/octet-stream", "etag": "f205307ef9f50594c4b86d9c246bee86", "x-minio-internal-erasure-upgraded": "5->6", "x-minio-internal-inline-data": "true", + }, + Parts: []ObjectPartInfo{ + { + ETag: "", + Number: 1, + Size: 329289, + ActualSize: 329289, + ModTime: time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), + Index: []uint8(nil), + Checksums: map[string]string(nil), + }, + }, + Erasure: ErasureInfo{ + Algorithm: "ReedSolomon", + DataBlocks: 6, + ParityBlocks: 6, + BlockSize: 1048576, + Index: 1, + Distribution: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}, + Checksums: []ChecksumInfo{{PartNumber: 1, Algorithm: 0x3, Hash: []uint8{}}}, + }, + NumVersions: 1, + Idx: 0, + } + + fi2 := FileInfo{ + Volume: "mybucket", + Name: "myobject", + VersionID: "", + IsLatest: true, + Deleted: false, + DataDir: "6f5c106d-9d28-4c85-a7f4-eac56225876b", + ModTime: time.Date(2023, time.March, 15, 19, 57, 30, 492530160, time.UTC), + Size: 329289, + Mode: 0x0, + WrittenByVersion: 0x63c77756, + Metadata: map[string]string{"content-type": "application/octet-stream", "etag": "f205307ef9f50594c4b86d9c246bee86", "x-minio-internal-inline-data": "true"}, + Parts: []ObjectPartInfo{ + { + ETag: "", + Number: 1, + Size: 329289, + ActualSize: 329289, + ModTime: time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), + Index: []uint8(nil), + Checksums: map[string]string(nil), + }, + }, + Erasure: ErasureInfo{ + Algorithm: "ReedSolomon", + DataBlocks: 7, + ParityBlocks: 5, + BlockSize: 1048576, + Index: 2, + Distribution: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}, + Checksums: []ChecksumInfo{ + {PartNumber: 1, Algorithm: 0x3, Hash: []uint8{}}, + }, + }, + NumVersions: 1, + Idx: 0, + } + + fiDel := FileInfo{ + Volume: "mybucket", + Name: "myobject", + VersionID: "", + IsLatest: true, + Deleted: true, + ModTime: time.Date(2023, time.March, 15, 19, 57, 30, 492530160, time.UTC), + Mode: 0x0, + WrittenByVersion: 0x63c77756, + NumVersions: 1, + Idx: 0, + } + + tests := []struct { + fi1, fi2 FileInfo + }{ + { + fi1: fi1, + fi2: fi2, + }, + { + fi1: fi1, + fi2: fiDel, + }, + } + for idx, test := range tests { + var metaArr []FileInfo + for i := 0; i < 12; i++ { + fi := test.fi1 + if i%2 == 0 { + fi = test.fi2 + } + metaArr = append(metaArr, fi) + } + + parities := listObjectParities(metaArr, make([]error, len(metaArr))) + parity := commonParity(parities, 5) + var match int + for _, fi := range metaArr { + if fi.Erasure.ParityBlocks == parity { + match++ + } + } + if match < len(metaArr)-parity { + t.Fatalf("Test %d: Expected %d drives with parity=%d, but got %d", idx, len(metaArr)-parity, parity, match) + } + } +} diff --git a/cmd/erasure-healing_test.go b/cmd/erasure-healing_test.go index ff60da72b..e243affe8 100644 --- a/cmd/erasure-healing_test.go +++ b/cmd/erasure-healing_test.go @@ -33,6 +33,7 @@ import ( "github.com/dustin/go-humanize" uuid2 "github.com/google/uuid" "github.com/minio/madmin-go/v2" + "github.com/minio/minio/internal/config/storageclass" ) // Tests isObjectDangling function @@ -563,6 +564,17 @@ func TestHealingDanglingObject(t *testing.T) { resetGlobalHealState() defer resetGlobalHealState() + // Set globalStoragClass.STANDARD to EC:4 for this test + saveSC := globalStorageClass + defer func() { + globalStorageClass = saveSC + }() + globalStorageClass = storageclass.Config{ + Standard: storageclass.StorageClass{ + Parity: 4, + }, + } + nDisks := 16 fsDirs, err := getRandomDisks(nDisks) if err != nil { @@ -577,6 +589,8 @@ func TestHealingDanglingObject(t *testing.T) { t.Fatal(err) } + setObjectLayer(objLayer) + bucket := getRandomBucketName() object := getRandomObjectName() data := bytes.Repeat([]byte("a"), 128*1024) diff --git a/cmd/erasure-metadata.go b/cmd/erasure-metadata.go index 623971c17..4471927a3 100644 --- a/cmd/erasure-metadata.go +++ b/cmd/erasure-metadata.go @@ -444,20 +444,33 @@ func writeUniqueFileInfo(ctx context.Context, disks []StorageAPI, bucket, prefix return evalDisks(disks, mErrs), err } -func commonParity(parities []int) int { +func commonParity(parities []int, defaultParityCount int) int { + N := len(parities) + occMap := make(map[int]int) for _, p := range parities { occMap[p]++ } var maxOcc, commonParity int - for parity, occ := range occMap { if parity == -1 { // Ignore non defined parity continue } - if occ >= maxOcc { + + readQuorum := N - parity + if defaultParityCount > 0 && parity == 0 { + // In this case, parity == 0 implies that this object version is a + // delete marker + readQuorum = N/2 + 1 + } + if occ < readQuorum { + // Ignore this parity since we don't have enough shards for read quorum + continue + } + + if occ > maxOcc { maxOcc = occ commonParity = parity } @@ -510,8 +523,7 @@ func objectQuorumFromMeta(ctx context.Context, partsMetaData []FileInfo, errs [] } parities := listObjectParities(partsMetaData, errs) - parityBlocks := commonParity(parities) - + parityBlocks := commonParity(parities, defaultParityCount) if parityBlocks < 0 { return -1, -1, errErasureReadQuorum }