You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by op...@apache.org on 2017/11/17 01:23:52 UTC
hbase git commit: HBASE-19252 Move the transform logic of FilterList
into transformCell() method to avoid extra ref to question cell
Repository: hbase
Updated Branches:
refs/heads/master 5b13b624b -> d72649283
HBASE-19252 Move the transform logic of FilterList into transformCell() method to avoid extra ref to question cell
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/d7264928
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/d7264928
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/d7264928
Branch: refs/heads/master
Commit: d726492838729a6a0312baa826ea0999253e81db
Parents: 5b13b62
Author: huzheng <op...@gmail.com>
Authored: Tue Nov 14 15:04:48 2017 +0800
Committer: openinx <op...@gmail.com>
Committed: Fri Nov 17 09:26:04 2017 +0800
----------------------------------------------------------------------
.../apache/hadoop/hbase/filter/FilterList.java | 19 -------
.../hadoop/hbase/filter/FilterListBase.java | 55 +++++++-------------
.../hadoop/hbase/filter/FilterListWithAND.java | 22 +++-----
.../hadoop/hbase/filter/FilterListWithOR.java | 18 +++----
.../hadoop/hbase/filter/TestFilterList.java | 54 +++++++++++++++++++
5 files changed, 89 insertions(+), 79 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/d7264928/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java
----------------------------------------------------------------------
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 4f9a8d8..3b6455e 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
@@ -165,25 +165,6 @@ final public class FilterList extends FilterBase {
return filterListBase.transformCell(c);
}
- /**
- * Internal implementation of {@link #filterCell(Cell)}. Compared to the
- * {@link #filterCell(Cell)} method, this method accepts an additional parameter named
- * transformedCell. This parameter indicates the initial value of transformed cell before this
- * filter operation. <br/>
- * 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 internalFilterCell(Cell c, Cell transformedCell) throws IOException {
- return this.filterListBase.internalFilterCell(c, transformedCell);
- }
-
@Override
@Deprecated
public ReturnCode filterKeyValue(final Cell c) throws IOException {
http://git-wip-us.apache.org/repos/asf/hbase/blob/d7264928/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java
----------------------------------------------------------------------
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 4087437..e02f7e2 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
@@ -26,8 +26,6 @@ import java.util.List;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellComparator;
-import org.apache.hadoop.hbase.CellUtil;
-import org.apache.hadoop.hbase.KeyValueUtil;
import org.apache.yetus.audience.InterfaceAudience;
/**
@@ -38,17 +36,12 @@ import org.apache.yetus.audience.InterfaceAudience;
public abstract class FilterListBase extends FilterBase {
private static final int MAX_LOG_FILTERS = 5;
protected final ArrayList<Filter> filters;
-
- /** Reference Cell used by {@link #transformCell(Cell)} for validation purpose. */
- protected Cell referenceCell = null;
-
/**
- * When filtering a given Cell in {@link #filterCell(Cell)}, this stores the transformed Cell
- * to be returned by {@link #transformCell(Cell)}. Individual filters transformation are applied
- * only when the filter includes the Cell. Transformations are composed in the order specified by
- * {@link #filters}.
+ * For each sub-filter in filter list, we save a boolean flag to indicate that whether the return
+ * code of filterCell(c) for sub-filter is INCLUDE* (INCLUDE, INCLUDE_AND_NEXT_COL,
+ * INCLUDE_AND_SEEK_NEXT_ROW) case. if true, we need to transform cell for the sub-filter.
*/
- protected Cell transformedCell = null;
+ protected ArrayList<Boolean> subFiltersIncludedCell;
public FilterListBase(List<Filter> filters) {
reversed = checkAndGetReversed(filters, reversed);
@@ -90,43 +83,35 @@ public abstract class FilterListBase extends FilterBase {
return reversed ? -1 * cmp : cmp;
}
+ /**
+ * 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. (HBASE-18879).
+ * @param c The cell in question.
+ * @return the transformed cell.
+ * @throws IOException
+ */
@Override
public Cell transformCell(Cell c) throws IOException {
if (isEmpty()) {
return super.transformCell(c);
}
- if (!CellUtil.equals(c, referenceCell)) {
- throw new IllegalStateException(
- "Reference Cell: " + this.referenceCell + " does not match: " + c);
+ Cell transformed = c;
+ for (int i = 0, n = filters.size(); i < n; i++) {
+ if (subFiltersIncludedCell.get(i)) {
+ transformed = filters.get(i).transformCell(transformed);
+ }
}
- // Copy transformedCell into a new cell and reset transformedCell & referenceCell to null for
- // Java GC optimization
- Cell cell = KeyValueUtil.copyToNewKeyValue(this.transformedCell);
- this.transformedCell = null;
- this.referenceCell = null;
- return cell;
+ return transformed;
}
- /**
- * Internal implementation of {@link #filterCell(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#internalFilterCell(Cell, Cell)
- */
- abstract ReturnCode internalFilterCell(Cell c, Cell transformedCell) throws IOException;
-
@Override
public ReturnCode filterKeyValue(final Cell c) throws IOException {
return filterCell(c);
}
- @Override
- public ReturnCode filterCell(final Cell c) throws IOException {
- return internalFilterCell(c, c);
- }
-
/**
* Filters that never filter by modifying the returned List of Cells can inherit this
* implementation that does nothing. {@inheritDoc}
http://git-wip-us.apache.org/repos/asf/hbase/blob/d7264928/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java
----------------------------------------------------------------------
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 9d4e619..9f2ca21 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
@@ -24,6 +24,7 @@ import org.apache.yetus.audience.InterfaceAudience;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
/**
@@ -37,6 +38,10 @@ public class FilterListWithAND extends FilterListBase {
public FilterListWithAND(List<Filter> filters) {
super(filters);
+ // For FilterList with AND, when call FL's transformCell(), we should transform cell for all
+ // sub-filters (because all sub-filters return INCLUDE*). So here, fill this array with true. we
+ // keep this in FilterListWithAND for abstracting the transformCell() in FilterListBase.
+ subFiltersIncludedCell = new ArrayList<>(Collections.nCopies(filters.size(), true));
}
@Override
@@ -45,6 +50,7 @@ public class FilterListWithAND extends FilterListBase {
throw new IllegalArgumentException("Filters in the list must have the same reversed flag");
}
this.filters.addAll(filters);
+ this.subFiltersIncludedCell.addAll(Collections.nCopies(filters.size(), true));
}
@Override
@@ -149,13 +155,11 @@ public class FilterListWithAND extends FilterListBase {
}
@Override
- ReturnCode internalFilterCell(Cell c, Cell transformedCell) throws IOException {
+ public ReturnCode filterCell(Cell c) throws IOException {
if (isEmpty()) {
return ReturnCode.INCLUDE;
}
ReturnCode rc = ReturnCode.INCLUDE;
- Cell transformed = transformedCell;
- this.referenceCell = c;
this.seekHintFilters.clear();
for (int i = 0, n = filters.size(); i < n; i++) {
Filter filter = filters.get(i);
@@ -163,23 +167,13 @@ public class FilterListWithAND extends FilterListBase {
return ReturnCode.NEXT_ROW;
}
ReturnCode localRC;
- if (filter instanceof FilterList) {
- localRC = ((FilterList) filter).internalFilterCell(c, transformed);
- } else {
- localRC = filter.filterCell(c);
- }
+ localRC = filter.filterCell(c);
rc = mergeReturnCode(rc, localRC);
- // For INCLUDE* case, we need to update the transformed cell.
- if (isInReturnCodes(localRC, ReturnCode.INCLUDE, ReturnCode.INCLUDE_AND_NEXT_COL,
- ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW)) {
- transformed = filter.transformCell(transformed);
- }
if (localRC == ReturnCode.SEEK_NEXT_USING_HINT) {
seekHintFilters.add(filter);
}
}
- this.transformedCell = transformed;
if (!seekHintFilters.isEmpty()) {
return ReturnCode.SEEK_NEXT_USING_HINT;
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/d7264928/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java
----------------------------------------------------------------------
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 51886bc..064dd83 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
@@ -48,6 +48,7 @@ public class FilterListWithOR extends FilterListBase {
super(filters);
prevFilterRCList = new ArrayList<>(Collections.nCopies(filters.size(), null));
prevCellList = new ArrayList<>(Collections.nCopies(filters.size(), null));
+ subFiltersIncludedCell = new ArrayList<>(Collections.nCopies(filters.size(), false));
}
@Override
@@ -56,6 +57,7 @@ public class FilterListWithOR extends FilterListBase {
throw new IllegalArgumentException("Filters in the list must have the same reversed flag");
}
this.filters.addAll(filters);
+ this.subFiltersIncludedCell.addAll(Collections.nCopies(filters.size(), false));
this.prevFilterRCList.addAll(Collections.nCopies(filters.size(), null));
this.prevCellList.addAll(Collections.nCopies(filters.size(), null));
}
@@ -246,16 +248,15 @@ public class FilterListWithOR extends FilterListBase {
}
@Override
- ReturnCode internalFilterCell(Cell c, Cell transformCell) throws IOException {
+ public ReturnCode filterCell(Cell c) throws IOException {
if (isEmpty()) {
return ReturnCode.INCLUDE;
}
ReturnCode rc = null;
boolean everyFilterReturnHint = true;
- Cell transformed = transformCell;
- this.referenceCell = c;
for (int i = 0, n = filters.size(); i < n; i++) {
Filter filter = filters.get(i);
+ subFiltersIncludedCell.set(i, false);
Cell prevCell = this.prevCellList.get(i);
ReturnCode prevCode = this.prevFilterRCList.get(i);
@@ -264,12 +265,7 @@ public class FilterListWithOR extends FilterListBase {
continue;
}
- ReturnCode localRC;
- if (filter instanceof FilterList) {
- localRC = ((FilterList) filter).internalFilterCell(c, transformed);
- } else {
- localRC = filter.filterCell(c);
- }
+ ReturnCode localRC = filter.filterCell(c);
// Update previous return code and previous cell for filter[i].
updatePrevFilterRCList(i, localRC);
@@ -284,11 +280,10 @@ public class FilterListWithOR extends FilterListBase {
// For INCLUDE* case, we need to update the transformed cell.
if (isInReturnCodes(localRC, ReturnCode.INCLUDE, ReturnCode.INCLUDE_AND_NEXT_COL,
ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW)) {
- transformed = filter.transformCell(transformed);
+ subFiltersIncludedCell.set(i, true);
}
}
- this.transformedCell = transformed;
if (everyFilterReturnHint) {
return ReturnCode.SEEK_NEXT_USING_HINT;
} else if (rc == null) {
@@ -303,6 +298,7 @@ public class FilterListWithOR extends FilterListBase {
public void reset() throws IOException {
for (int i = 0, n = filters.size(); i < n; i++) {
filters.get(i).reset();
+ subFiltersIncludedCell.set(i, false);
prevFilterRCList.set(i, null);
prevCellList.set(i, null);
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/d7264928/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java
----------------------------------------------------------------------
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 16a57fb..fa5981e 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
@@ -960,5 +960,59 @@ public class TestFilterList {
filter.filterCell(kv2);
assertEquals(2, mockNextRowFilter.getHitCount());
}
+
+ private static class TransformFilter extends FilterBase {
+ private ReturnCode targetRetCode;
+ private boolean transformed = false;
+
+ public TransformFilter(ReturnCode targetRetCode) {
+ this.targetRetCode = targetRetCode;
+ }
+
+ @Override
+ public ReturnCode filterCell(final Cell v) throws IOException {
+ return targetRetCode;
+ }
+
+ @Override
+ public Cell transformCell(Cell c) throws IOException {
+ transformed = true;
+ return super.transformCell(c);
+ }
+
+ public boolean getTransformed() {
+ return this.transformed;
+ }
+ }
+
+ @Test
+ public void testTransformCell() throws IOException {
+ KeyValue kv =
+ new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("cf"), Bytes.toBytes("column1"), 1,
+ Bytes.toBytes("value"));
+
+ // case MUST_PASS_ONE
+ TransformFilter filter1 = new TransformFilter(ReturnCode.INCLUDE);
+ TransformFilter filter2 = new TransformFilter(ReturnCode.NEXT_ROW);
+ TransformFilter filter3 = new TransformFilter(ReturnCode.SEEK_NEXT_USING_HINT);
+ FilterList filterList = new FilterList(Operator.MUST_PASS_ONE, filter1, filter2, filter3);
+ Assert.assertEquals(ReturnCode.INCLUDE, filterList.filterCell(kv));
+ Assert.assertEquals(kv, filterList.transformCell(kv));
+ Assert.assertEquals(true, filter1.getTransformed());
+ Assert.assertEquals(false, filter2.getTransformed());
+ Assert.assertEquals(false, filter3.getTransformed());
+
+ // case MUST_PASS_ALL
+ filter1 = new TransformFilter(ReturnCode.INCLUDE);
+ filter2 = new TransformFilter(ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW);
+ filter3 = new TransformFilter(ReturnCode.INCLUDE_AND_NEXT_COL);
+ filterList = new FilterList(Operator.MUST_PASS_ALL, filter1, filter2, filter3);
+
+ Assert.assertEquals(ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW, filterList.filterCell(kv));
+ Assert.assertEquals(kv, filterList.transformCell(kv));
+ Assert.assertEquals(true, filter1.getTransformed());
+ Assert.assertEquals(true, filter2.getTransformed());
+ Assert.assertEquals(true, filter3.getTransformed());
+ }
}