HBASE-27637 Zero length value would cause value compressor read nothing and not advance the position of the InputStream

Author: Apache9

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressionContext.java

@@ -126,7 +126,16 @@ public int decompress(InputStream in, int inLength, byte[] outArray, int outOffs
       // Caller must handle short reads.
       // With current Hadoop compression codecs all 'outLength' bytes are read in here, so not
       // an issue for now.
-      return compressedIn.read(outArray, outOffset, outLength);
+      int read = compressedIn.read(outArray, outOffset, outLength);
+      if (outLength == 0 && read == 0) {
+        // The BufferedInputStream will return earlier and skip reading anything if outLength = 0,
+        // but in fact for an empty value, the compressed output still contains some metadata so the
+        // compressed size is not 0, so here we need to manually skip inLength bytes otherwise the
+        // next read on this stream will start from an invalid position and cause critical problem,
+        // such as data loss when splitting wal or replicating wal.
+        in.skip(inLength);
+      }
+      return read;
     }
 
     public void clear() {

hbase-server/src/test/java/org/apache/hadoop/hbase/wal/CompressedWALTestBase.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.wal;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -26,8 +28,9 @@
 import java.util.TreeMap;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellBuilderFactory;
+import org.apache.hadoop.hbase.CellBuilderType;
 import org.apache.hadoop.hbase.HBaseTestingUtil;
-import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
@@ -82,33 +85,42 @@ public void doTest(TableName tableName) throws Exception {
 
     for (int i = 0; i < total; i++) {
       WALEdit kvs = new WALEdit();
-      kvs.add(new KeyValue(row, family, Bytes.toBytes(i), value));
+      kvs.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY).setType(Cell.Type.Put)
+        .setRow(row).setFamily(family).setQualifier(Bytes.toBytes(i)).setValue(value).build());
+      kvs.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY)
+        .setType(Cell.Type.DeleteFamily).setRow(row).setFamily(family).build());
       wal.appendData(regionInfo, new WALKeyImpl(regionInfo.getEncodedNameAsBytes(), tableName,
         System.currentTimeMillis(), mvcc, scopes), kvs);
+      wal.sync();
     }
-    wal.sync();
     final Path walPath = AbstractFSWALProvider.getCurrentFileName(wal);
     wals.shutdown();
 
     // Confirm the WAL can be read back
-    WAL.Reader reader = wals.createReader(TEST_UTIL.getTestFileSystem(), walPath);
-    int count = 0;
-    WAL.Entry entry = new WAL.Entry();
-    while (reader.next(entry) != null) {
-      count++;
-      List<Cell> cells = entry.getEdit().getCells();
-      assertTrue("Should be one KV per WALEdit", cells.size() == 1);
-      for (Cell cell : cells) {
-        assertTrue("Incorrect row", Bytes.equals(cell.getRowArray(), cell.getRowOffset(),
-          cell.getRowLength(), row, 0, row.length));
-        assertTrue("Incorrect family", Bytes.equals(cell.getFamilyArray(), cell.getFamilyOffset(),
-          cell.getFamilyLength(), family, 0, family.length));
-        assertTrue("Incorrect value", Bytes.equals(cell.getValueArray(), cell.getValueOffset(),
-          cell.getValueLength(), value, 0, value.length));
+    try (WAL.Reader reader = wals.createReader(TEST_UTIL.getTestFileSystem(), walPath)) {
+      int count = 0;
+      WAL.Entry entry = new WAL.Entry();
+      while (reader.next(entry) != null) {
+        count++;
+        List<Cell> cells = entry.getEdit().getCells();
+        assertThat("Should be two KVs per WALEdit", cells, hasSize(2));
+        Cell putCell = cells.get(0);
+        assertEquals(Cell.Type.Put, putCell.getType());
+        assertTrue("Incorrect row", Bytes.equals(putCell.getRowArray(), putCell.getRowOffset(),
+          putCell.getRowLength(), row, 0, row.length));
+        assertTrue("Incorrect family", Bytes.equals(putCell.getFamilyArray(),
+          putCell.getFamilyOffset(), putCell.getFamilyLength(), family, 0, family.length));
+        assertTrue("Incorrect value", Bytes.equals(putCell.getValueArray(),
+          putCell.getValueOffset(), putCell.getValueLength(), value, 0, value.length));
+
+        Cell deleteCell = cells.get(1);
+        assertEquals(Cell.Type.DeleteFamily, deleteCell.getType());
+        assertTrue("Incorrect row", Bytes.equals(deleteCell.getRowArray(),
+          deleteCell.getRowOffset(), deleteCell.getRowLength(), row, 0, row.length));
+        assertTrue("Incorrect family", Bytes.equals(deleteCell.getFamilyArray(),
+          deleteCell.getFamilyOffset(), deleteCell.getFamilyLength(), family, 0, family.length));
       }
+      assertEquals("Should have read back as many KVs as written", total, count);
     }
-    assertEquals("Should have read back as many KVs as written", total, count);
-    reader.close();
   }
-
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestCompressedWALValueCompression.java

@@ -20,7 +20,6 @@
 import java.util.List;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseCommonTestingUtil;
-import org.apache.hadoop.hbase.HBaseTestingUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.io.compress.Compression;
@@ -46,9 +45,7 @@ public class TestCompressedWALValueCompression extends CompressedWALTestBase {
   public static final HBaseClassTestRule CLASS_RULE =
     HBaseClassTestRule.forClass(TestCompressedWALValueCompression.class);
 
-  private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
-
-  @Parameters
+  @Parameters(name = "{index}: compression={0}")
   public static List<Object[]> params() {
     return HBaseCommonTestingUtil.COMPRESSION_ALGORITHMS_PARAMETERIZED;
   }
@@ -81,5 +78,4 @@ public void test() throws Exception {
     TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
     doTest(tableName);
   }
-
 }