commonParity should pick readable FileInfo (#17032)

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)
+		}
+	}
+}

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)

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
 	}