From d9a88955a50c0b9b5a91c3bfc13569ed23ab8f1e Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Thu, 5 Dec 2019 15:40:24 +0900 Subject: [PATCH] HBASE-23370 PageFilter returns extra records even when page is filled within a region --- .../hbase/regionserver/RSRpcServices.java | 7 +-- .../hadoop/hbase/filter/TestPageFilter.java | 44 ++++++++++++++++++- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 379e254010..97ef45aa9a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -3549,11 +3549,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler, addResults(builder, results, (HBaseRpcController) controller, RegionReplicaUtil.isDefaultReplica(region.getRegionInfo()), isClientCellBlockSupport(context)); - if (scanner.isFilterDone() && results.isEmpty()) { - // If the scanner's filter - if any - is done with the scan - // only set moreResults to false if the results is empty. This is used to keep compatible - // with the old scan implementation where we just ignore the returned results if moreResults - // is false. Can remove the isEmpty check after we get rid of the old implementation. + if (scanner.isFilterDone()) { + // If the scanner's filter - if any - is done with the scan, set moreResults to false. builder.setMoreResults(false); } // Later we may close the scanner depending on this flag so here we need to make sure that we diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPageFilter.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPageFilter.java index 5ab0ff971c..2b6ce697d7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPageFilter.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPageFilter.java @@ -23,8 +23,15 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.testclassification.FilterTests; -import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hbase.thirdparty.com.google.common.collect.Iterators; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -32,9 +39,11 @@ import org.junit.experimental.categories.Category; /** * Tests for the page filter */ -@Category({FilterTests.class, SmallTests.class}) +@Category({FilterTests.class, MediumTests.class}) public class TestPageFilter { + private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestPageFilter.class); @@ -67,6 +76,37 @@ public class TestPageFilter { pageSizeTests(newFilter); } + @Test + public void testShouldNotAdvanceToNextRegionWhenPageFilled() throws Exception { + final TableName tableName = TableName.valueOf("page-filter-test"); + final byte[] cf = "d".getBytes(); + final byte[][] splitKeys = new byte[][]{"1".getBytes(), "2".getBytes()}; + + try { + TEST_UTIL.startMiniCluster(); + Table table = TEST_UTIL.createTable(tableName, cf, splitKeys); + String[] rowkeys = new String[]{ + "00", "01", "02", + "10", "11", "12", + "20", "21", "22" + }; + for (String rowkey : rowkeys) { + Put put = new Put(rowkey.getBytes()); + put.addColumn(cf, "foo".getBytes(), "bar".getBytes()); + table.put(put); + } + + Scan scan = new Scan(); + scan.setFilter(new PageFilter(2)); + scan.setCaching(rowkeys.length); + try (ResultScanner scanner = table.getScanner(scan)) { + assertEquals(2, Iterators.size(scanner.iterator())); + } + } finally { + TEST_UTIL.shutdownMiniCluster(); + } + } + private void pageSizeTests(Filter f) throws Exception { testFiltersBeyondPageSize(f, ROW_LIMIT); } -- 2.24.0