From 7a2da02e6dcd4f1a5c5d204baf0647bc17421cd0 Mon Sep 17 00:00:00 2001 From: huzheng Date: Tue, 17 Oct 2017 19:25:23 +0800 Subject: [PATCH] HBASE-18368 FilterList with multiple FamilyFilters concatenated by OR does not work Signed-off-by: Guanghao Zhang --- .../apache/hadoop/hbase/filter/Filter.java | 10 ++++--- .../hadoop/hbase/filter/FilterListWithOR.java | 10 +++++-- .../hadoop/hbase/filter/TestFilterList.java | 26 +++++++++++++++++++ .../hbase/filter/TestFilterListOnMini.java | 7 +++-- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java index 70c68b6a5a8..a92ea0be898 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java @@ -172,8 +172,12 @@ public abstract class Filter { */ NEXT_COL, /** - * Done with columns, skip to next row. Note that filterRow() will - * still be called. + * Seek to next row in current family. It may still pass a cell whose family is different but + * row is the same as previous cell to {@link #filterKeyValue(Cell)} , even if we get a NEXT_ROW + * returned for previous cell. For more details see HBASE-18368.
+ * Once reset() method was invoked, then we switch to the next row for all family, and you can + * catch the event by invoking CellUtils.matchingRows(previousCell, currentCell).
+ * Note that filterRow() will still be called.
*/ NEXT_ROW, /** @@ -181,7 +185,7 @@ public abstract class Filter { */ SEEK_NEXT_USING_HINT, /** - * Include KeyValue and done with row, seek to next. + * Include KeyValue and done with row, seek to next. See NEXT_ROW. */ INCLUDE_AND_SEEK_NEXT_ROW, } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java index bac9023fd5d..31e2a554964 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java @@ -74,7 +74,12 @@ public class FilterListWithOR extends FilterListBase { * as the previous cell even if filter-A has NEXT_COL returned for the previous cell. So we should * save the previous cell and the return code list when checking previous cell for every filter in * filter list, and verify if currentCell fit the previous return code, if fit then pass the - * currentCell to the corresponding filter. (HBASE-17678) + * currentCell to the corresponding filter. (HBASE-17678)
+ * Note that: In StoreScanner level, NEXT_ROW will skip to the next row in current family, and in + * RegionScanner level, NEXT_ROW will skip to the next row in current family and switch to the + * next family for RegionScanner, INCLUDE_AND_NEXT_ROW is the same. so we should pass current cell + * to the filter, if row mismatch or row match but column family mismatch. (HBASE-18368) + * @see org.apache.hadoop.hbase.filter.Filter.ReturnCode */ private boolean shouldPassCurrentCellToFilter(Cell prevCell, Cell currentCell, int filterIdx) throws IOException { @@ -94,7 +99,8 @@ public class FilterListWithOR extends FilterListBase { return !CellUtil.matchingRowColumn(prevCell, currentCell); case NEXT_ROW: case INCLUDE_AND_SEEK_NEXT_ROW: - return !CellUtil.matchingRows(prevCell, currentCell); + return !CellUtil.matchingRows(prevCell, currentCell) + || !CellUtil.matchingFamily(prevCell, currentCell); default: throw new IllegalStateException("Received code is not valid."); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java index 4c67c599d0f..d420b02dc5d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java @@ -936,5 +936,31 @@ public class TestFilterList { assertEquals(ReturnCode.INCLUDE, filterList.filterKeyValue(kv)); assertEquals(kv, filterList.transformCell(kv)); } + + private static class MockNextRowFilter extends FilterBase { + private int hitCount = 0; + + public ReturnCode filterKeyValue(Cell v) throws IOException { + hitCount++; + return ReturnCode.NEXT_ROW; + } + + public int getHitCount() { + return hitCount; + } + } + + @Test + public void testRowCountFilter() throws IOException { + KeyValue kv1 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("fam1"), Bytes.toBytes("a"), 1, + Bytes.toBytes("value")); + KeyValue kv2 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("fam2"), Bytes.toBytes("a"), 2, + Bytes.toBytes("value")); + MockNextRowFilter mockNextRowFilter = new MockNextRowFilter(); + FilterList filter = new FilterList(Operator.MUST_PASS_ONE, mockNextRowFilter); + filter.filterKeyValue(kv1); + filter.filterKeyValue(kv2); + assertEquals(2, mockNextRowFilter.getHitCount()); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java index 590b26e829c..2a13ac8a663 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java @@ -36,10 +36,10 @@ import org.junit.experimental.categories.Category; import org.junit.rules.TestName; /** - * Tests filter Lists in ways that rely on a MiniCluster. - * Where possible, favor tests in TestFilterList and TestFilterFromRegionSide instead. + * Tests filter Lists in ways that rely on a MiniCluster. Where possible, favor tests in + * TestFilterList and TestFilterFromRegionSide instead. */ -@Category({MediumTests.class, FilterTests.class}) +@Category({ MediumTests.class, FilterTests.class }) public class TestFilterListOnMini { private static final Log LOG = LogFactory.getLog(TestFilterListOnMini.class); @@ -58,7 +58,6 @@ public class TestFilterListOnMini { TEST_UTIL.shutdownMiniCluster(); } - @Ignore("HBASE-18410 Should not merge without this test running.") @Test public void testFiltersWithOR() throws Exception { TableName tn = TableName.valueOf(name.getMethodName());