Prevent unnecessary verification of parity blocks while reading (#4683)

* Prevent unnecessary verification of parity blocks while reading erasure
  coded file.
* Update klauspost/reedsolomon and just only reconstruct data blocks while
  reading (prevent unnecessary parity block reconstruction)
* Remove Verification of (all) reconstructed Data and Parity blocks since
  in our case we are protected by bit rot protection. And even if the
  verification would fail (essentially impossible) there is no way to
  definitively say whether the data is still correct or not, so this call
  make no sense for our use case.

cmd/erasure-createfile_test.go

@@ -174,11 +174,11 @@ func TestErasureEncode(t *testing.T) {
 			reedsolomon.ErrInvShardNum,
 		},
 		// TestCase - 8.
-		// test case with data + parity blocks > 255.
+		// test case with data + parity blocks > 256.
 		// expected to fail with Error Max Shard number.
 		{
 			[]byte("1"),
-			128,
+			129,
 			128,
 			false,
 			reedsolomon.ErrMaxShardNum,

cmd/erasure-healfile.go

@@ -53,8 +53,8 @@ func erasureHealFile(latestDisks []StorageAPI, outDatedDisks []StorageAPI, volum
 			}
 		}
 
-		// Reconstruct missing data.
-		err := decodeData(enBlocks, dataBlocks, parityBlocks)
+		// Reconstruct any missing data and parity blocks.
+		err := decodeDataAndParity(enBlocks, dataBlocks, parityBlocks)
 		if err != nil {
 			return nil, err
 		}

cmd/erasure-readfile.go

@@ -17,7 +17,6 @@
 package cmd
 
 import (
-	"errors"
 	"io"
 	"sync"
 
@@ -272,7 +271,7 @@ func erasureReadFile(writer io.Writer, disks []StorageAPI, volume, path string,
 		// If we have all the data blocks no need to decode, continue to write.
 		if !isSuccessDataBlocks(enBlocks, dataBlocks) {
 			// Reconstruct the missing data blocks.
-			if err := decodeData(enBlocks, dataBlocks, parityBlocks); err != nil {
+			if err := decodeMissingData(enBlocks, dataBlocks, parityBlocks); err != nil {
 				return bytesWritten, err
 			}
 		}
@@ -314,31 +313,26 @@ func erasureReadFile(writer io.Writer, disks []StorageAPI, volume, path string,
 	return bytesWritten, nil
 }
 
-// decodeData - decode encoded blocks.
-func decodeData(enBlocks [][]byte, dataBlocks, parityBlocks int) error {
-	// Initialized reedsolomon.
+// decodeMissingData - decode any missing data blocks.
+func decodeMissingData(enBlocks [][]byte, dataBlocks, parityBlocks int) error {
+	// Initialize reedsolomon.
+	rs, err := reedsolomon.New(dataBlocks, parityBlocks)
+	if err != nil {
+		return traceError(err)
+	}
+
+	// Reconstruct any missing data blocks.
+	return rs.ReconstructData(enBlocks)
+}
+
+// decodeDataAndParity - decode all encoded data and parity blocks.
+func decodeDataAndParity(enBlocks [][]byte, dataBlocks, parityBlocks int) error {
+	// Initialize reedsolomon.
 	rs, err := reedsolomon.New(dataBlocks, parityBlocks)
 	if err != nil {
 		return traceError(err)
 	}
 
 	// Reconstruct encoded blocks.
-	err = rs.Reconstruct(enBlocks)
-	if err != nil {
-		return traceError(err)
-	}
-
-	// Verify reconstructed blocks (parity).
-	ok, err := rs.Verify(enBlocks)
-	if err != nil {
-		return traceError(err)
-	}
-	if !ok {
-		// Blocks cannot be reconstructed, corrupted data.
-		err = errors.New("Verification failed after reconstruction, data likely corrupted")
-		return traceError(err)
-	}
-
-	// Success.
-	return nil
+	return rs.Reconstruct(enBlocks)
 }

cmd/erasure_test.go

@@ -124,26 +124,54 @@ func TestErasureDecode(t *testing.T) {
 			// Data block size.
 			blockSize := len(data)
 
-			// Generates encoded data based on type of testCase function.
-			encodedData := testCase.enFn(data, dataBlocks, parityBlocks)
+			// Test decoder for just the missing data blocks
+			{
+				// Generates encoded data based on type of testCase function.
+				encodedData := testCase.enFn(data, dataBlocks, parityBlocks)
 
-			// Decodes the data.
-			err := decodeData(encodedData, dataBlocks, parityBlocks)
-			if err != nil && testCase.shouldPass {
-				t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
+				// Decodes the data.
+				err := decodeMissingData(encodedData, dataBlocks, parityBlocks)
+				if err != nil && testCase.shouldPass {
+					t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
+				}
+
+				// Proceed to extract the data blocks.
+				decodedDataWriter := new(bytes.Buffer)
+				_, err = writeDataBlocks(decodedDataWriter, encodedData, dataBlocks, 0, int64(blockSize))
+				if err != nil && testCase.shouldPass {
+					t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
+				}
+
+				// Validate if decoded data is what we expected.
+				if bytes.Equal(decodedDataWriter.Bytes(), data) != testCase.shouldPass {
+					err := errUnexpected
+					t.Errorf("Test %d: Expected to pass by failed instead %s", i+1, err)
+				}
 			}
 
-			// Proceed to extract the data blocks.
-			decodedDataWriter := new(bytes.Buffer)
-			_, err = writeDataBlocks(decodedDataWriter, encodedData, dataBlocks, 0, int64(blockSize))
-			if err != nil && testCase.shouldPass {
-				t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
-			}
+			// Test decoder for all missing data and parity blocks
+			{
+				// Generates encoded data based on type of testCase function.
+				encodedData := testCase.enFn(data, dataBlocks, parityBlocks)
 
-			// Validate if decoded data is what we expected.
-			if bytes.Equal(decodedDataWriter.Bytes(), data) != testCase.shouldPass {
-				err := errUnexpected
-				t.Errorf("Test %d: Expected to pass by failed instead %s", i+1, err)
+				// Decodes the data.
+				err := decodeDataAndParity(encodedData, dataBlocks, parityBlocks)
+				if err != nil && testCase.shouldPass {
+					t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
+				}
+
+				// Proceed to extract the data blocks.
+				decodedDataWriter := new(bytes.Buffer)
+				_, err = writeDataBlocks(decodedDataWriter, encodedData, dataBlocks, 0, int64(blockSize))
+				if err != nil && testCase.shouldPass {
+					t.Errorf("Test %d: Expected to pass by failed instead with %s", i+1, err)
+				}
+
+				// Validate if decoded data is what we expected.
+				if bytes.Equal(decodedDataWriter.Bytes(), data) != testCase.shouldPass {
+					err := errUnexpected
+					t.Errorf("Test %d: Expected to pass by failed instead %s", i+1, err)
+				}
 			}
 		}
 	}