From 59f8ef3777a53126d788c4b138da517bd91de972 Mon Sep 17 00:00:00 2001 From: openinx Date: Sat, 21 Oct 2017 21:38:22 +0800 Subject: [PATCH] HBASE-19057 Merge branch HBASE-18410 to master branch --- .../org/apache/hadoop/hbase/filter/FilterList.java | 19 ++++++++++-- .../apache/hadoop/hbase/filter/FilterListBase.java | 16 ++++++++-- .../hadoop/hbase/filter/FilterListWithAND.java | 12 ++------ .../hadoop/hbase/filter/FilterListWithOR.java | 34 ++++++++++------------ .../apache/hadoop/hbase/filter/TestFilterList.java | 4 ++- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java index 79f3e78..0f3057f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java @@ -164,8 +164,23 @@ final public class FilterList extends FilterBase { return filterListBase.transformCell(c); } - ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformedCell) throws IOException { - return this.filterListBase.internalFilterKeyValue(c, currentTransformedCell); + /** + * Internal implementation of {@link #filterKeyValue(Cell)}. Compared to the + * {@link #filterKeyValue(Cell)} method, this method accepts an additional parameter named + * currentTransformedCell. This parameter indicates the initial value of transformed cell before + * this filter operation.
+ * For FilterList, we can consider a filter list as a node in a tree. sub-filters of the filter + * list are children of the relative node. The logic of transforming cell of a filter list, well, + * we can consider it as the process of post-order tree traverse. For a node , Before we traverse + * the current child, we should set the traverse result (transformed cell) of previous node(s) as + * the initial value. so the additional currentTransformedCell parameter is needed (HBASE-18879). + * @param c The cell in question. + * @param transformedCell The transformed cell of previous filter(s) + * @return ReturnCode of this filter operation. + * @throws IOException + */ + ReturnCode internalFilterKeyValue(Cell c, Cell transformedCell) throws IOException { + return this.filterListBase.internalFilterKeyValue(c, transformedCell); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java index 06da6be..1d30fd69 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java @@ -107,8 +107,20 @@ public abstract class FilterListBase extends FilterBase { return cell; } - abstract ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformedCell) - throws IOException; + /** + * Internal implementation of {@link #filterKeyValue(Cell)} + * @param c The cell in question. + * @param transformedCell The transformed cell of previous filter(s) + * @return ReturnCode of this filter operation. + * @throws IOException + * @see org.apache.hadoop.hbase.filter.FilterList#internalFilterKeyValue(Cell, Cell) + */ + abstract ReturnCode internalFilterKeyValue(Cell c, Cell transformedCell) throws IOException; + + @Override + public ReturnCode filterKeyValue(Cell c) throws IOException { + return internalFilterKeyValue(c, c); + } /** * Filters that never filter by modifying the returned List of Cells can inherit this diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java index 4909dfd..09efb82 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java @@ -186,11 +186,6 @@ public class FilterListWithAND extends FilterListBase { } @Override - public ReturnCode filterKeyValue(Cell c) throws IOException { - return internalFilterKeyValue(c, c); - } - - @Override public void reset() throws IOException { for (int i = 0, n = filters.size(); i < n; i++) { filters.get(i).reset(); @@ -206,7 +201,7 @@ public class FilterListWithAND extends FilterListBase { boolean retVal = false; for (int i = 0, n = filters.size(); i < n; i++) { Filter filter = filters.get(i); - if (filter.filterAllRemaining() || filter.filterRowKey(rowKey, offset, length)) { + if (filter.filterRowKey(rowKey, offset, length)) { retVal = true; } } @@ -221,7 +216,7 @@ public class FilterListWithAND extends FilterListBase { boolean retVal = false; for (int i = 0, n = filters.size(); i < n; i++) { Filter filter = filters.get(i); - if (filter.filterAllRemaining() || filter.filterRowKey(firstRowCell)) { + if (filter.filterRowKey(firstRowCell)) { retVal = true; } } @@ -262,9 +257,6 @@ public class FilterListWithAND extends FilterListBase { } Cell maxHint = null; for (Filter filter : seekHintFilter) { - if (filter.filterAllRemaining()) { - continue; - } Cell curKeyHint = filter.getNextCellHint(currentCell); if (maxHint == null) { maxHint = curKeyHint; 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 31e2a55..6ee301a 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 @@ -81,9 +81,8 @@ public class FilterListWithOR extends FilterListBase { * 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 { - ReturnCode prevCode = this.prevFilterRCList.get(filterIdx); + private boolean shouldPassCurrentCellToFilter(Cell prevCell, Cell currentCell, ReturnCode prevCode) + throws IOException { if (prevCell == null || prevCode == null) { return true; } @@ -96,11 +95,13 @@ public class FilterListWithOR extends FilterListBase { return nextHintCell == null || this.compareCell(currentCell, nextHintCell) >= 0; case NEXT_COL: case INCLUDE_AND_NEXT_COL: - return !CellUtil.matchingRowColumn(prevCell, currentCell); + // Once row changed, reset() will clear prevCells, so we need not to compare their rows + // because rows are the same here. + return !CellUtil.matchingColumn(prevCell, currentCell); case NEXT_ROW: case INCLUDE_AND_SEEK_NEXT_ROW: - return !CellUtil.matchingRows(prevCell, currentCell) - || !CellUtil.matchingFamily(prevCell, currentCell); + // As described above, rows are definitely the same, so we only compare the family. + return !CellUtil.matchingFamily(prevCell, currentCell); default: throw new IllegalStateException("Received code is not valid."); } @@ -181,8 +182,10 @@ public class FilterListWithOR extends FilterListBase { if (isInReturnCodes(rc, ReturnCode.INCLUDE)) { return ReturnCode.INCLUDE; } - if (isInReturnCodes(rc, ReturnCode.INCLUDE_AND_NEXT_COL, - ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW)) { + if (isInReturnCodes(rc, ReturnCode.NEXT_COL, ReturnCode.NEXT_ROW)) { + return ReturnCode.NEXT_COL; + } + if (isInReturnCodes(rc, ReturnCode.INCLUDE_AND_NEXT_COL, ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW)) { return ReturnCode.INCLUDE_AND_NEXT_COL; } if (isInReturnCodes(rc, ReturnCode.SKIP, ReturnCode.SEEK_NEXT_USING_HINT)) { @@ -254,7 +257,8 @@ public class FilterListWithOR extends FilterListBase { Filter filter = filters.get(i); Cell prevCell = this.prevCellList.get(i); - if (filter.filterAllRemaining() || !shouldPassCurrentCellToFilter(prevCell, c, i)) { + ReturnCode prevRC = this.prevFilterRCList.get(i); + if (filter.filterAllRemaining() || !shouldPassCurrentCellToFilter(prevCell, c, prevRC)) { everyFilterReturnHint = false; continue; } @@ -295,11 +299,6 @@ public class FilterListWithOR extends FilterListBase { } @Override - public ReturnCode filterKeyValue(Cell c) throws IOException { - return internalFilterKeyValue(c, c); - } - - @Override public void reset() throws IOException { for (int i = 0, n = filters.size(); i < n; i++) { filters.get(i).reset(); @@ -316,7 +315,7 @@ public class FilterListWithOR extends FilterListBase { boolean retVal = true; for (int i = 0, n = filters.size(); i < n; i++) { Filter filter = filters.get(i); - if (!filter.filterAllRemaining() && !filter.filterRowKey(rowKey, offset, length)) { + if (!filter.filterRowKey(rowKey, offset, length)) { retVal = false; } } @@ -331,7 +330,7 @@ public class FilterListWithOR extends FilterListBase { boolean retVal = true; for (int i = 0, n = filters.size(); i < n; i++) { Filter filter = filters.get(i); - if (!filter.filterAllRemaining() && !filter.filterRowKey(firstRowCell)) { + if (!filter.filterRowKey(firstRowCell)) { retVal = false; } } @@ -373,9 +372,6 @@ public class FilterListWithOR extends FilterListBase { Cell minKeyHint = null; // If any condition can pass, we need to keep the min hint for (int i = 0, n = filters.size(); i < n; i++) { - if (filters.get(i).filterAllRemaining()) { - continue; - } Cell curKeyHint = filters.get(i).getNextCellHint(currentCell); if (curKeyHint == null) { // If we ever don't have a hint and this is must-pass-one, then no hint 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 c55094d..e4c6638 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 @@ -326,7 +326,7 @@ public class TestFilterList { * @throws Exception */ @Test - public void testFilterListWithInclusiveStopFilteMustPassOne() throws Exception { + public void testFilterListWithInclusiveStopFilterMustPassOne() throws Exception { byte[] r1 = Bytes.toBytes("Row1"); byte[] r11 = Bytes.toBytes("Row11"); byte[] r2 = Bytes.toBytes("Row2"); @@ -698,6 +698,7 @@ public class TestFilterList { filter.filterKeyValue(kv3); assertFalse(mockFilter.didCellPassToTheFilter); + filter.reset(); mockFilter.didCellPassToTheFilter = false; filter.filterKeyValue(kv4); assertTrue(mockFilter.didCellPassToTheFilter); @@ -715,6 +716,7 @@ public class TestFilterList { filter.filterKeyValue(kv3); assertFalse(mockFilter.didCellPassToTheFilter); + filter.reset(); mockFilter.didCellPassToTheFilter = false; filter.filterKeyValue(kv4); assertTrue(mockFilter.didCellPassToTheFilter); -- 2.3.2 (Apple Git-55)