(hbase) branch branch-3 updated: HBASE-28482 Reverse scan with tags throws ArrayIndexOutOfBoundsException with DBE in setCurrentBlock flow (#5792)

[bb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bbeaudreault pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-3 by this push:
     new ddcc9da9f58 HBASE-28482 Reverse scan with tags throws ArrayIndexOutOfBoundsException with DBE in setCurrentBlock flow (#5792)
ddcc9da9f58 is described below

commit ddcc9da9f588bcee3b2c56620b3f1f7d4027e218
Author: Vineet Kumar Maheshwari <vi...@users.noreply.github.com>
AuthorDate: Sun Apr 28 19:56:13 2024 +0530

    HBASE-28482 Reverse scan with tags throws ArrayIndexOutOfBoundsException with DBE in setCurrentBlock flow (#5792)
    
    Signed-off-by: Pankaj Kumar <pa...@apache.org>
    Signed-off-by: Bryan Beaudreault <bb...@apache.org>
---
 .../io/encoding/BufferedDataBlockEncoder.java      | 10 ++++--
 .../apache/hadoop/hbase/regionserver/TestTags.java | 36 +++++++++++++++-------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
index f998f40d68b..1794422d5cd 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
@@ -834,6 +834,13 @@ abstract class BufferedDataBlockEncoder extends AbstractDataBlockEncoder {
     public void setCurrentBuffer(ByteBuff buffer) {
       if (this.tagCompressionContext != null) {
         this.tagCompressionContext.clear();
+
+        // Prior seekToKeyInBlock may have reset this to false if we fell back to previous
+        // seeker state. This is an optimization so we don't have to uncompress tags again when
+        // reading last state.
+        // In seekBefore flow, if block change happens then rewind is not called and
+        // setCurrentBuffer is called, so need to uncompress any tags we see.
+        current.uncompressTags = true;
       }
       currentBuffer = buffer;
       current.currentBuffer = currentBuffer;
@@ -876,9 +883,6 @@ abstract class BufferedDataBlockEncoder extends AbstractDataBlockEncoder {
         // reading last state.
         // In case of rewind, we are starting from the beginning of the buffer, so we need
         // to uncompress any tags we see.
-        // It may make sense to reset this in setCurrentBuffer as well, but we seem to only call
-        // setCurrentBuffer after StoreFileScanner.seekAtOrAfter which calls next to consume the
-        // seeker state. Rewind is called by seekBefore, which doesn't and leaves us in this state.
         current.uncompressTags = true;
       }
       decodeFirst();
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
index 3d45fc56503..45b927c07af 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
@@ -124,32 +124,46 @@ public class TestTags {
 
     try (Connection connection = ConnectionFactory.createConnection(conf)) {
       for (DataBlockEncoding encoding : DataBlockEncoding.values()) {
-        testReverseScanWithDBE(connection, encoding, family);
+        testReverseScanWithDBE(connection, encoding, family, HConstants.DEFAULT_BLOCKSIZE, 10);
       }
     }
   }
 
-  private void testReverseScanWithDBE(Connection conn, DataBlockEncoding encoding, byte[] family)
-    throws IOException {
+  /**
+   * Test that we can do reverse scans when writing tags and using DataBlockEncoding. Fails with an
+   * exception for PREFIX, DIFF, and FAST_DIFF
+   */
+  @Test
+  public void testReverseScanWithDBEWhenCurrentBlockUpdates() throws IOException {
+    byte[] family = Bytes.toBytes("0");
+
+    Configuration conf = new Configuration(TEST_UTIL.getConfiguration());
+    conf.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 1);
+
+    try (Connection connection = ConnectionFactory.createConnection(conf)) {
+      for (DataBlockEncoding encoding : DataBlockEncoding.values()) {
+        testReverseScanWithDBE(connection, encoding, family, 1024, 30000);
+      }
+    }
+  }
+
+  private void testReverseScanWithDBE(Connection conn, DataBlockEncoding encoding, byte[] family,
+    int blockSize, int maxRows) throws IOException {
     LOG.info("Running test with DBE={}", encoding);
     TableName tableName = TableName.valueOf(TEST_NAME.getMethodName() + "-" + encoding);
-    TEST_UTIL.createTable(TableDescriptorBuilder.newBuilder(tableName)
-      .setColumnFamily(
-        ColumnFamilyDescriptorBuilder.newBuilder(family).setDataBlockEncoding(encoding).build())
-      .build(), null);
+    TEST_UTIL.createTable(
+      TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(ColumnFamilyDescriptorBuilder
+        .newBuilder(family).setDataBlockEncoding(encoding).setBlocksize(blockSize).build()).build(),
+      null);
 
     Table table = conn.getTable(tableName);
 
-    int maxRows = 10;
     byte[] val1 = new byte[10];
     byte[] val2 = new byte[10];
     Bytes.random(val1);
     Bytes.random(val2);
 
     for (int i = 0; i < maxRows; i++) {
-      if (i == maxRows / 2) {
-        TEST_UTIL.flush(tableName);
-      }
       table.put(new Put(Bytes.toBytes(i)).addColumn(family, Bytes.toBytes(1), val1)
         .addColumn(family, Bytes.toBytes(2), val2).setTTL(600_000));
     }