HADOOP-11921. Enhance tests for erasure coders. Contributed by Kai Zheng.

hadoop-common-project/hadoop-common/CHANGES-HDFS-EC-7285.txt

@@ -44,3 +44,5 @@
     HADOOP-11818. Minor improvements for erasurecode classes. (Rakesh R via Kai Zheng)
 
     HADOOP-11841. Remove unused ecschema-def.xml files.  (szetszwo)
+
+    HADOOP-11921. Enhance tests for erasure coders. (Kai Zheng via Zhe Zhang)

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/TestCoderBase.java

@@ -49,15 +49,15 @@ public abstract class TestCoderBase {
    * Prepare before running the case.
    * @param numDataUnits
    * @param numParityUnits
-   * @param erasedIndexes
+   * @param erasedDataIndexes
    */
   protected void prepare(Configuration conf, int numDataUnits,
-                         int numParityUnits, int[] erasedIndexes) {
+                         int numParityUnits, int[] erasedDataIndexes) {
     this.conf = conf;
     this.numDataUnits = numDataUnits;
     this.numParityUnits = numParityUnits;
-    this.erasedDataIndexes = erasedIndexes != null ?
+    this.erasedDataIndexes = erasedDataIndexes != null ?
-        erasedIndexes : new int[] {0};
+        erasedDataIndexes : new int[] {0};
   }
 
   /**
@@ -82,15 +82,19 @@ public abstract class TestCoderBase {
   }
 
   /**
-   * Adjust and return erased indexes based on the array of the input chunks (
+   * Adjust and return erased indexes altogether, including erased data indexes
-   * parity chunks + data chunks).
+   * and parity indexes.
-   * @return
+   * @return erased indexes altogether
    */
   protected int[] getErasedIndexesForDecoding() {
     int[] erasedIndexesForDecoding = new int[erasedDataIndexes.length];
+
+    int idx = 0;
+
     for (int i = 0; i < erasedDataIndexes.length; i++) {
-      erasedIndexesForDecoding[i] = erasedDataIndexes[i] + numParityUnits;
+      erasedIndexesForDecoding[idx ++] = erasedDataIndexes[i] + numParityUnits;
     }
+
     return erasedIndexesForDecoding;
   }
 
@@ -116,30 +120,23 @@ public abstract class TestCoderBase {
   }
 
   /**
-   * Have a copy of the data chunks that's to be erased thereafter. The copy
+   * Erase chunks to test the recovering of them. Before erasure clone them
-   * will be used to compare and verify with the to be recovered chunks.
+   * first so could return them.
    * @param dataChunks
-   * @return
+   * @return clone of erased chunks
    */
-  protected ECChunk[] copyDataChunksToErase(ECChunk[] dataChunks) {
+  protected ECChunk[] backupAndEraseChunks(ECChunk[] dataChunks) {
-    ECChunk[] copiedChunks = new ECChunk[erasedDataIndexes.length];
+    ECChunk[] toEraseChunks = new ECChunk[erasedDataIndexes.length];
+
+    int idx = 0;
 
-    int j = 0;
     for (int i = 0; i < erasedDataIndexes.length; i++) {
-      copiedChunks[j ++] = cloneChunkWithData(dataChunks[erasedDataIndexes[i]]);
+      ECChunk chunk = dataChunks[erasedDataIndexes[i]];
+      toEraseChunks[idx ++] = cloneChunkWithData(chunk);
+      eraseDataFromChunk(chunk);
     }
 
-    return copiedChunks;
+    return toEraseChunks;
-  }
-
-  /**
-   * Erase some data chunks to test the recovering of them
-   * @param dataChunks
-   */
-  protected void eraseSomeDataBlocks(ECChunk[] dataChunks) {
-    for (int i = 0; i < erasedDataIndexes.length; i++) {
-      eraseDataFromChunk(dataChunks[erasedDataIndexes[i]]);
-    }
   }
 
   /**
@@ -277,6 +274,7 @@ public abstract class TestCoderBase {
    */
   protected ECChunk[] prepareOutputChunksForDecoding() {
     ECChunk[] chunks = new ECChunk[erasedDataIndexes.length];
+
     for (int i = 0; i < chunks.length; i++) {
       chunks[i] = allocateOutputChunk();
     }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestErasureCoderBase.java

@@ -29,6 +29,9 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
   protected Class<? extends ErasureCoder> encoderClass;
   protected Class<? extends ErasureCoder> decoderClass;
 
+  private ErasureCoder encoder;
+  private ErasureCoder decoder;
+
   protected int numChunksInBlock = 16;
 
   /**
@@ -54,39 +57,27 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
    */
   protected void testCoding(boolean usingDirectBuffer) {
     this.usingDirectBuffer = usingDirectBuffer;
-
+    prepareCoders();
-    ErasureCoder encoder = createEncoder();
 
     // Generate data and encode
     ECBlockGroup blockGroup = prepareBlockGroupForEncoding();
     // Backup all the source chunks for later recovering because some coders
     // may affect the source data.
-    TestBlock[] clonedDataBlocks = cloneBlocksWithData((TestBlock[])
+    TestBlock[] clonedDataBlocks = cloneBlocksWithData((TestBlock[]) blockGroup.getDataBlocks());
-        blockGroup.getDataBlocks());
-    // Make a copy of a strip for later comparing
-    TestBlock[] toEraseBlocks = copyDataBlocksToErase(clonedDataBlocks);
 
     ErasureCodingStep codingStep;
-    try {
     codingStep = encoder.calculateCoding(blockGroup);
     performCodingStep(codingStep);
-    } finally {
+    // Erase specified sources but return copies of them for later comparing
-      encoder.release();
+    TestBlock[] backupBlocks = backupAndEraseBlocks(clonedDataBlocks);
-    }
-    // Erase the copied sources
-    eraseSomeDataBlocks(clonedDataBlocks);
 
     // Decode
     blockGroup = new ECBlockGroup(clonedDataBlocks, blockGroup.getParityBlocks());
-    ErasureCoder decoder = createDecoder();
-    try {
     codingStep = decoder.calculateCoding(blockGroup);
     performCodingStep(codingStep);
-    } finally {
+
-      decoder.release();
-    }
     // Compare
-    compareAndVerify(toEraseBlocks, codingStep.getOutputBlocks());
+    compareAndVerify(backupBlocks, codingStep.getOutputBlocks());
   }
 
   /**
@@ -129,8 +120,7 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
   protected void compareAndVerify(ECBlock[] erasedBlocks,
                                   ECBlock[] recoveredBlocks) {
     for (int i = 0; i < erasedBlocks.length; ++i) {
-      compareAndVerify(((TestBlock) erasedBlocks[i]).chunks,
+      compareAndVerify(((TestBlock) erasedBlocks[i]).chunks, ((TestBlock) recoveredBlocks[i]).chunks);
-          ((TestBlock) recoveredBlocks[i]).chunks);
     }
   }
 
@@ -151,6 +141,16 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
     return encoder;
   }
 
+  private void prepareCoders() {
+    if (encoder == null) {
+      encoder = createEncoder();
+    }
+
+    if (decoder == null) {
+      decoder = createDecoder();
+    }
+  }
+
   /**
    * Create the erasure decoder for the test.
    * @return
@@ -201,6 +201,26 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
     return new TestBlock(chunks);
   }
 
+  /**
+   * Erase blocks to test the recovering of them. Before erasure clone them
+   * first so could return themselves.
+   * @param dataBlocks
+   * @return clone of erased dataBlocks
+   */
+  protected TestBlock[] backupAndEraseBlocks(TestBlock[] dataBlocks) {
+    TestBlock[] toEraseBlocks = new TestBlock[erasedDataIndexes.length];
+
+    int idx = 0;
+
+    for (int i = 0; i < erasedDataIndexes.length; i++) {
+      TestBlock block = dataBlocks[erasedDataIndexes[i]];
+      toEraseBlocks[idx ++] = cloneBlockWithData(block);
+      eraseDataFromBlock(block);
+    }
+
+    return toEraseBlocks;
+  }
+
   /**
    * Copy those data blocks that's to be erased for later comparing and
    * verifying.
@@ -255,22 +275,9 @@ public abstract class TestErasureCoderBase extends TestCoderBase {
   }
 
   /**
-   * Erase some data blocks specified by the indexes from the data blocks.
+   * Erase data from a block.
-   * @param dataBlocks
    */
-  protected void eraseSomeDataBlocks(TestBlock[] dataBlocks) {
+  protected void eraseDataFromBlock(TestBlock theBlock) {
-    for (int i = 0; i < erasedDataIndexes.length; ++i) {
-      eraseDataFromBlock(dataBlocks, erasedDataIndexes[i]);
-    }
-  }
-
-  /**
-   * Erase data from a block specified by erased index.
-   * @param blocks
-   * @param erasedIndex
-   */
-  protected void eraseDataFromBlock(TestBlock[] blocks, int erasedIndex) {
-    TestBlock theBlock = blocks[erasedIndex];
     eraseDataFromChunks(theBlock.chunks);
     theBlock.setErased(true);
   }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestRSErasureCoder.java

@@ -40,19 +40,18 @@ public class TestRSErasureCoder extends TestErasureCoderBase {
   }
 
   @Test
-  public void testCodingNoDirectBuffer_10x4() {
+  public void testCodingNoDirectBuffer_10x4_erasing_d0() {
-    prepare(null, 10, 4, null);
+    prepare(null, 10, 4, new int[] {0});
+    /**
+     * Doing twice to test if the coders can be repeatedly reused. This matters
+     * as the underlying coding buffers are shared, which may have bugs.
+     */
+    testCoding(false);
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4() {
+  public void testCodingDirectBufferWithConf_10x4_erasing_d0() {
-    prepare(null, 10, 4, null);
-    testCoding(true);
-  }
-
-  @Test
-  public void testCodingDirectBufferWithConf_10x4() {
     /**
      * This tests if the two configuration items work or not.
      */
@@ -61,31 +60,62 @@ public class TestRSErasureCoder extends TestErasureCoderBase {
         RSRawErasureCoderFactory.class.getCanonicalName());
     conf.setBoolean(
         CommonConfigurationKeys.IO_ERASURECODE_CODEC_RS_USEXOR_KEY, false);
-    prepare(conf, 10, 4, null);
+
+    prepare(conf, 10, 4, new int[]{0});
+
     testCoding(true);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4_erasure_of_2_4() {
+  public void testCodingDirectBuffer_10x4_erasing_d2() {
+    prepare(null, 10, 4, new int[] {2});
+    testCoding(true);
+    testCoding(true);
+  }
+
+  @Test
+  public void testCodingDirectBuffer_10x4_erasing_d0() {
+    prepare(null, 10, 4, new int[] {0});
+    testCoding(true);
+    testCoding(true);
+  }
+
+  @Test
+  public void testCodingBothBuffers_10x4_erasing_d0() {
+    prepare(null, 10, 4, new int[] {0});
+
+    /**
+     * Doing in mixed buffer usage model to test if the coders can be repeatedly
+     * reused with different buffer usage model. This matters as the underlying
+     * coding buffers are shared, which may have bugs.
+     */
+    testCoding(true);
+    testCoding(false);
+    testCoding(true);
+    testCoding(false);
+  }
+
+  @Test
+  public void testCodingDirectBuffer_10x4_erasure_of_d2_d4() {
     prepare(null, 10, 4, new int[] {2, 4});
     testCoding(true);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4_erasing_all() {
+  public void testCodingDirectBuffer_10x4_erasing_d0_d1() {
-    prepare(null, 10, 4, new int[] {0, 1, 2, 3});
+    prepare(null, 10, 4, new int[] {0, 1});
     testCoding(true);
   }
 
   @Test
-  public void testCodingNoDirectBuffer_3x3() {
+  public void testCodingNoDirectBuffer_3x3_erasing_d0() {
-    prepare(null, 3, 3, null);
+    prepare(null, 3, 3, new int[] {0});
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer_3x3() {
+  public void testCodingDirectBuffer_3x3_erasing_d0() {
-    prepare(null, 3, 3, null);
+    prepare(null, 3, 3, new int[] {0});
     testCoding(true);
   }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/coder/TestXORCoder.java

@@ -32,19 +32,33 @@ public class TestXORCoder extends TestErasureCoderBase {
 
     this.numDataUnits = 10;
     this.numParityUnits = 1;
-    this.erasedDataIndexes = new int[] {0};
-
     this.numChunksInBlock = 10;
   }
 
   @Test
-  public void testCodingNoDirectBuffer() {
+  public void testCodingNoDirectBuffer_erasing_d0() {
+    prepare(null, 10, 1, new int[] {0});
+
+    /**
+     * Doing twice to test if the coders can be repeatedly reused. This matters
+     * as the underlying coding buffers are shared, which may have bugs.
+     */
+    testCoding(false);
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer() {
+  public void testCodingBothBuffers_erasing_d5() {
-    testCoding(true);
+    prepare(null, 10, 1, new int[]{5});
-  }
 
+    /**
+     * Doing in mixed buffer usage model to test if the coders can be repeatedly
+     * reused with different buffer usage model. This matters as the underlying
+     * coding buffers are shared, which may have bugs.
+     */
+    testCoding(true);
+    testCoding(false);
+    testCoding(true);
+    testCoding(false);
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/TestRSRawCoder.java

@@ -17,26 +17,13 @@
  */
 package org.apache.hadoop.io.erasurecode.rawcoder;
 
-import org.apache.hadoop.io.erasurecode.ECChunk;
-import org.apache.hadoop.io.erasurecode.rawcoder.util.RSUtil;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.nio.ByteBuffer;
-
 /**
- * Test raw Reed-solomon encoding and decoding.
+ * Test raw Reed-solomon coder implemented in Java.
  */
-public class TestRSRawCoder extends TestRawCoderBase {
+public class TestRSRawCoder extends TestRSRawCoderBase {
-
-  private static int symbolSize = 0;
-  private static int symbolMax = 0;
-
-  static {
-    symbolSize = (int) Math.round(Math.log(
-        RSUtil.GF.getFieldSize()) / Math.log(2));
-    symbolMax = (int) Math.pow(2, symbolSize);
-  }
 
   @Before
   public void setup() {
@@ -45,49 +32,66 @@ public class TestRSRawCoder extends TestRawCoderBase {
   }
 
   @Test
-  public void testCodingNoDirectBuffer_10x4() {
+  public void testCodingNoDirectBuffer_10x4_erasing_d0() {
-    prepare(null, 10, 4, null);
+    prepare(null, 10, 4, new int[] {0});
+    /**
+     * Doing twice to test if the coders can be repeatedly reused. This matters
+     * as the underlying coding buffers are shared, which may have bugs.
+     */
+    testCoding(false);
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4() {
+  public void testCodingDirectBuffer_10x4_erasing_d2() {
-    prepare(null, 10, 4, null);
+    prepare(null, 10, 4, new int[] {2});
+    testCoding(true);
     testCoding(true);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4_erasure_of_2_4() {
+  public void testCodingDirectBuffer_10x4_erasing_d0() {
+    prepare(null, 10, 4, new int[] {0});
+    testCoding(true);
+    testCoding(true);
+  }
+
+  @Test
+  public void testCodingBothBuffers_10x4_erasing_d0() {
+    prepare(null, 10, 4, new int[] {0});
+
+    /**
+     * Doing in mixed buffer usage model to test if the coders can be repeatedly
+     * reused with different buffer usage model. This matters as the underlying
+     * coding buffers are shared, which may have bugs.
+     */
+    testCoding(true);
+    testCoding(false);
+    testCoding(true);
+    testCoding(false);
+  }
+
+  @Test
+  public void testCodingDirectBuffer_10x4_erasure_of_d2_d4() {
     prepare(null, 10, 4, new int[] {2, 4});
     testCoding(true);
   }
 
   @Test
-  public void testCodingDirectBuffer_10x4_erasing_all() {
+  public void testCodingDirectBuffer_10x4_erasing_d0_d1() {
-    prepare(null, 10, 4, new int[] {0, 1, 2, 3});
+    prepare(null, 10, 4, new int[] {0, 1});
     testCoding(true);
   }
 
   @Test
-  public void testCodingNoDirectBuffer_3x3() {
+  public void testCodingNoDirectBuffer_3x3_erasing_d0() {
-    prepare(null, 3, 3, null);
+    prepare(null, 3, 3, new int[] {0});
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer_3x3() {
+  public void testCodingDirectBuffer_3x3_erasing_d0() {
-    prepare(null, 3, 3, null);
+    prepare(null, 3, 3, new int[] {0});
     testCoding(true);
   }
-
-  @Override
-  protected ECChunk generateDataChunk() {
-    ByteBuffer buffer = allocateOutputBuffer();
-    for (int i = 0; i < chunkSize; i++) {
-      buffer.put((byte) RAND.nextInt(symbolMax));
-    }
-    buffer.flip();
-
-    return new ECChunk(buffer);
-  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/TestRSRawCoderBase.java

@@ -0,0 +1,51 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.io.erasurecode.rawcoder;
+
+import org.apache.hadoop.io.erasurecode.ECChunk;
+import org.apache.hadoop.io.erasurecode.rawcoder.util.RSUtil;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Test base for raw Reed-solomon coders.
+ */
+public abstract class TestRSRawCoderBase extends TestRawCoderBase {
+
+  private static int symbolSize = 0;
+  private static int symbolMax = 0;
+
+  static {
+    symbolSize = (int) Math.round(Math.log(
+        RSUtil.GF.getFieldSize()) / Math.log(2));
+    symbolMax = (int) Math.pow(2, symbolSize);
+  }
+
+  @Override
+  protected ECChunk generateDataChunk() {
+    ByteBuffer buffer = allocateOutputBuffer();
+    for (int i = 0; i < chunkSize; i++) {
+      buffer.put((byte) RAND.nextInt(symbolMax));
+    }
+    buffer.flip();
+
+    return new ECChunk(buffer);
+  }
+}

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/TestRawCoderBase.java

@@ -26,6 +26,8 @@ import org.apache.hadoop.io.erasurecode.TestCoderBase;
 public abstract class TestRawCoderBase extends TestCoderBase {
   protected Class<? extends RawErasureEncoder> encoderClass;
   protected Class<? extends RawErasureDecoder> decoderClass;
+  private RawErasureEncoder encoder;
+  private RawErasureDecoder decoder;
 
   /**
    * Generating source data, encoding, recovering and then verifying.
@@ -37,40 +39,41 @@ public abstract class TestRawCoderBase extends TestCoderBase {
    */
   protected void testCoding(boolean usingDirectBuffer) {
     this.usingDirectBuffer = usingDirectBuffer;
+    prepareCoders();
 
     // Generate data and encode
     ECChunk[] dataChunks = prepareDataChunksForEncoding();
     ECChunk[] parityChunks = prepareParityChunksForEncoding();
-    RawErasureEncoder encoder = createEncoder();
 
     // Backup all the source chunks for later recovering because some coders
     // may affect the source data.
     ECChunk[] clonedDataChunks = cloneChunksWithData(dataChunks);
-    // Make a copy of a strip for later comparing
-    ECChunk[] toEraseDataChunks = copyDataChunksToErase(clonedDataChunks);
 
-    try {
     encoder.encode(dataChunks, parityChunks);
-    } finally {
+
-      encoder.release();
+    // Backup and erase some chunks
-    }
+    ECChunk[] backupChunks = backupAndEraseChunks(clonedDataChunks);
-    // Erase the copied sources
-    eraseSomeDataBlocks(clonedDataChunks);
 
     // Decode
-    ECChunk[] inputChunks = prepareInputChunksForDecoding(clonedDataChunks,
+    ECChunk[] inputChunks = prepareInputChunksForDecoding(
-        parityChunks);
+        clonedDataChunks, parityChunks);
+
     ECChunk[] recoveredChunks = prepareOutputChunksForDecoding();
-    RawErasureDecoder decoder = createDecoder();
+
-    try {
+    decoder.decode(inputChunks, getErasedIndexesForDecoding(), recoveredChunks);
-      decoder.decode(inputChunks,
-          getErasedIndexesForDecoding(), recoveredChunks);
-    } finally {
-      decoder.release();
-    }
 
     // Compare
-    compareAndVerify(toEraseDataChunks, recoveredChunks);
+    compareAndVerify(backupChunks, recoveredChunks);
+  }
+
+  private void prepareCoders() {
+    if (encoder == null) {
+      encoder = createEncoder();
+    }
+
+    if (decoder == null) {
+      decoder = createDecoder();
+    }
   }
 
   /**

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/TestXORRawCoder.java

@@ -32,18 +32,32 @@ public class TestXORRawCoder extends TestRawCoderBase {
 
     this.numDataUnits = 10;
     this.numParityUnits = 1;
-
-    this.erasedDataIndexes = new int[] {0};
   }
 
   @Test
-  public void testCodingNoDirectBuffer() {
+  public void testCodingNoDirectBuffer_erasing_d0() {
+    prepare(null, 10, 1, new int[] {0});
+
+    /**
+     * Doing twice to test if the coders can be repeatedly reused. This matters
+     * as the underlying coding buffers are shared, which may have bugs.
+     */
+    testCoding(false);
     testCoding(false);
   }
 
   @Test
-  public void testCodingDirectBuffer() {
+  public void testCodingBothBuffers_erasing_d5() {
-    testCoding(true);
+    prepare(null, 10, 1, new int[]{5});
-  }
 
+    /**
+     * Doing in mixed buffer usage model to test if the coders can be repeatedly
+     * reused with different buffer usage model. This matters as the underlying
+     * coding buffers are shared, which may have bugs.
+     */
+    testCoding(true);
+    testCoding(false);
+    testCoding(true);
+    testCoding(false);
+  }
 }