HBASE-26533 KeyValueScanner might not be properly closed when using I…

…nternalScan.checkOnlyMemStore() (apache#3917) Signed-off-by: Duo Zhang <[email protected]>
flyMei · Dec 5, 2021 · 554580f · 554580f
1 parent b5b286d
commit 554580f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 0 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
@@ -464,6 +464,7 @@ protected List<KeyValueScanner> selectScannersFrom(HStore store,
     for (KeyValueScanner kvs : allScanners) {
       boolean isFile = kvs.isFileScanner();
       if ((!isFile && filesOnly) || (isFile && memOnly)) {
+        kvs.close();
         continue;
       }
 

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
@@ -22,6 +22,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
@@ -56,6 +57,7 @@
 import org.apache.hadoop.hbase.testclassification.RegionServerTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.CollectionBackedScanner;
 import org.apache.hadoop.hbase.util.EnvironmentEdge;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManagerTestHelper;
@@ -1080,4 +1082,46 @@ public void testReadVersionWithRawAndFilter() throws IOException {
       assertEquals(2, results.size());
     }
   }
+
+  @Test
+  public void testScannersClosedWhenCheckingOnlyMemStore() throws IOException {
+    class MyCollectionBackedScanner extends CollectionBackedScanner {
+      final boolean fileScanner;
+      boolean closed;
+
+      MyCollectionBackedScanner(boolean fileScanner) {
+        super(Collections.emptySortedSet());
+        this.fileScanner = fileScanner;
+      }
+
+      @Override
+      public boolean isFileScanner() {
+        return fileScanner;
+      }
+
+      @Override
+      public void close() {
+        super.close();
+        closed = true;
+      }
+    }
+
+    ScanInfo scanInfo = new ScanInfo(CONF, CF, 0, 1, Long.MAX_VALUE,
+      KeepDeletedCells.FALSE, HConstants.DEFAULT_BLOCKSIZE, 0
+      , CellComparator.getInstance(), false);
+    InternalScan scan = new InternalScan(new Scan());
+    scan.checkOnlyMemStore();
+    MyCollectionBackedScanner fileScanner = new MyCollectionBackedScanner(true);
+    MyCollectionBackedScanner memStoreScanner = new MyCollectionBackedScanner(false);
+    List<? extends KeyValueScanner> allScanners = Arrays.asList(fileScanner, memStoreScanner);
+
+    try (StoreScanner scanner = new StoreScanner(scan, scanInfo, null, allScanners)) {
+      List<KeyValueScanner> remaining = scanner.selectScannersFrom(null, allScanners);
+
+      assertEquals(1, remaining.size());
+      assertSame(memStoreScanner, remaining.get(0));
+      assertTrue(fileScanner.closed);
+      assertFalse(memStoreScanner.closed);
+    }
+  }
 }