Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
Umeshkumar9414 commented on code in PR #8464:
URL: https://github.com/apache/hbase/pull/8464#discussion_r3612821894
##
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java:
##
@@ -219,6 +223,85 @@ public enum ReturnCode {
*/
abstract public Cell getNextCellHint(final Cell currentCell) throws
IOException;
+ /**
+ * Provides a seek hint to bypass row-by-row scanning after {@link
#filterRowKey(Cell)} rejects a
+ * row. When {@code filterRowKey} returns {@code true} the scan pipeline
would normally iterate
+ * through every remaining cell in the rejected row one-by-one (via {@code
nextRow()}) before
+ * moving on. If the filter can determine a better forward position — for
example, the next range
+ * boundary in a {@code MultiRowRangeFilter} — it should return that target
cell here, allowing
+ * the scanner to seek directly past the unwanted rows.
+ *
+ * Contract:
+ *
+ * Only called after {@link #filterRowKey(Cell)} has returned {@code
true} for the same
+ * {@code firstRowCell}.
+ * Implementations may use state that was set during {@link
#filterRowKey(Cell)} (e.g. an
+ * updated range pointer), but must not invoke {@link
#filterCell(Cell)} logic —
+ * the caller guarantees that {@code filterCell} has not been called for
this row.
+ * The returned {@link Cell}, if non-null, must be an
+ * {@link org.apache.hadoop.hbase.ExtendedCell} because filters are
evaluated on the server
+ * side.
+ * Returning {@code null} (the default) falls through to the existing
{@code nextRow()}
+ * behaviour, preserving full backward compatibility.
+ * For reversed scans ({@link
org.apache.hadoop.hbase.client.Scan#isReversed()}), the hint
+ * must point to a smaller row key (earlier in reverse-scan
direction). The scanner
+ * validates hint direction and falls back to {@code nextRow()} if the hint
does not advance in
+ * the scan direction.
+ * Composite filter limitation: {@code FilterList},
{@code SkipFilter}, and
+ * {@code WhileMatchFilter} do not currently delegate this method to wrapped
sub-filters. Hints
+ * from filters used inside these wrappers will be silently ignored.
+ *
+ * @param firstRowCell the first cell encountered in the rejected row;
contains the row key that
+ * was passed to {@code filterRowKey}
+ * @return a {@link Cell} representing the earliest position the scanner
should seek to, or
+ * {@code null} if this filter cannot provide a better position than
a sequential skip
+ * @throws IOException in case an I/O or filter-specific failure needs to be
signaled
+ * @see #filterRowKey(Cell)
+ */
+ public Cell getHintForRejectedRow(final Cell firstRowCell) throws
IOException {
+return null;
+ }
+
+ /**
+ * Provides a seek hint for cells that are structurally skipped by the scan
pipeline
+ * before {@link #filterCell(Cell)} is ever reached. The pipeline
short-circuits on
+ * several criteria — time-range mismatch, column-set exclusion, and
version-limit exhaustion —
+ * and in each case the filter is bypassed entirely. When an implementation
can compute a
+ * meaningful forward position purely from the cell's coordinates (without
needing the
+ * {@code filterCell} call sequence), it should return that position here so
the scanner can seek
+ * ahead instead of advancing one cell at a time.
+ *
+ * Contract:
+ *
+ * May be called for cells that have never been passed
to
+ * {@link #filterCell(Cell)}.
+ * Implementations must not modify any filter state;
this method is treated
+ * as logically stateless. Only filters whose hint computation is based
solely on immutable
+ * configuration (e.g. a fixed column range or a fuzzy-row pattern) should
override this.
+ * The returned {@link Cell}, if non-null, must be an
+ * {@link org.apache.hadoop.hbase.ExtendedCell} because filters are
evaluated on the server
Review Comment:
does this comment related to ExtendedCell still apply ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
Umeshkumar9414 commented on code in PR #8464:
URL: https://github.com/apache/hbase/pull/8464#discussion_r3612821894
##
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java:
##
@@ -219,6 +223,85 @@ public enum ReturnCode {
*/
abstract public Cell getNextCellHint(final Cell currentCell) throws
IOException;
+ /**
+ * Provides a seek hint to bypass row-by-row scanning after {@link
#filterRowKey(Cell)} rejects a
+ * row. When {@code filterRowKey} returns {@code true} the scan pipeline
would normally iterate
+ * through every remaining cell in the rejected row one-by-one (via {@code
nextRow()}) before
+ * moving on. If the filter can determine a better forward position — for
example, the next range
+ * boundary in a {@code MultiRowRangeFilter} — it should return that target
cell here, allowing
+ * the scanner to seek directly past the unwanted rows.
+ *
+ * Contract:
+ *
+ * Only called after {@link #filterRowKey(Cell)} has returned {@code
true} for the same
+ * {@code firstRowCell}.
+ * Implementations may use state that was set during {@link
#filterRowKey(Cell)} (e.g. an
+ * updated range pointer), but must not invoke {@link
#filterCell(Cell)} logic —
+ * the caller guarantees that {@code filterCell} has not been called for
this row.
+ * The returned {@link Cell}, if non-null, must be an
+ * {@link org.apache.hadoop.hbase.ExtendedCell} because filters are
evaluated on the server
+ * side.
+ * Returning {@code null} (the default) falls through to the existing
{@code nextRow()}
+ * behaviour, preserving full backward compatibility.
+ * For reversed scans ({@link
org.apache.hadoop.hbase.client.Scan#isReversed()}), the hint
+ * must point to a smaller row key (earlier in reverse-scan
direction). The scanner
+ * validates hint direction and falls back to {@code nextRow()} if the hint
does not advance in
+ * the scan direction.
+ * Composite filter limitation: {@code FilterList},
{@code SkipFilter}, and
+ * {@code WhileMatchFilter} do not currently delegate this method to wrapped
sub-filters. Hints
+ * from filters used inside these wrappers will be silently ignored.
+ *
+ * @param firstRowCell the first cell encountered in the rejected row;
contains the row key that
+ * was passed to {@code filterRowKey}
+ * @return a {@link Cell} representing the earliest position the scanner
should seek to, or
+ * {@code null} if this filter cannot provide a better position than
a sequential skip
+ * @throws IOException in case an I/O or filter-specific failure needs to be
signaled
+ * @see #filterRowKey(Cell)
+ */
+ public Cell getHintForRejectedRow(final Cell firstRowCell) throws
IOException {
+return null;
+ }
+
+ /**
+ * Provides a seek hint for cells that are structurally skipped by the scan
pipeline
+ * before {@link #filterCell(Cell)} is ever reached. The pipeline
short-circuits on
+ * several criteria — time-range mismatch, column-set exclusion, and
version-limit exhaustion —
+ * and in each case the filter is bypassed entirely. When an implementation
can compute a
+ * meaningful forward position purely from the cell's coordinates (without
needing the
+ * {@code filterCell} call sequence), it should return that position here so
the scanner can seek
+ * ahead instead of advancing one cell at a time.
+ *
+ * Contract:
+ *
+ * May be called for cells that have never been passed
to
+ * {@link #filterCell(Cell)}.
+ * Implementations must not modify any filter state;
this method is treated
+ * as logically stateless. Only filters whose hint computation is based
solely on immutable
+ * configuration (e.g. a fixed column range or a fuzzy-row pattern) should
override this.
+ * The returned {@link Cell}, if non-null, must be an
+ * {@link org.apache.hadoop.hbase.ExtendedCell} because filters are
evaluated on the server
Review Comment:
does this comment still apply ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
virajjasani merged PR #7882: URL: https://github.com/apache/hbase/pull/7882 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
virajjasani commented on PR #7882: URL: https://github.com/apache/hbase/pull/7882#issuecomment-4413237067 ❌ | spotless | 0m 34s | patch has 23 errors when running spotless:check, run spotless:apply to fix -- | -- | -- | -- -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
shubham-roy commented on code in PR #7882:
URL: https://github.com/apache/hbase/pull/7882#discussion_r3194117328
##
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterHintForRejectedRow.java:
##
@@ -0,0 +1,361 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.filter;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.PrivateCellUtil;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.Durability;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.RegionInfoBuilder;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.testclassification.FilterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+/**
+ * Integration tests for HBASE-29974 Path 1: {@link
Filter#getHintForRejectedRow(Cell)} allows the
+ * scan pipeline to seek directly past rejected rows instead of iterating
through every cell in each
+ * rejected row one-by-one via {@code nextRow()}.
+ *
+ * Each test verifies two properties simultaneously:
+ *
+ * Correctness — the scan returns exactly the rows that are not
rejected by
+ * {@link Filter#filterRowKey(Cell)}, regardless of whether the hint path or
the legacy
+ * cell-iteration path is used.
+ * Efficiency — when a filter provides a hint that jumps over all N
rejected rows in one
+ * seek, {@code getHintForRejectedRow} is called exactly once (not N times),
proving that the
+ * scanner did not fall back to cell-by-cell iteration for the skipped
rows.
+ *
+ */
+@Category({ FilterTests.class, MediumTests.class })
+public class TestFilterHintForRejectedRow {
Review Comment:
`getHintForRejectedRow()` is also equally applicable for the phoenix cdc
based backup solution that we are developing. We identify row keys that were
mutated for the backup time range via CDC and then get information for the same
row keys. While skipping row keys that were not mutated
`getHintForRejectedRow()` comes into play.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
virajjasani commented on code in PR #7882:
URL: https://github.com/apache/hbase/pull/7882#discussion_r3103930261
##
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterHintForRejectedRow.java:
##
@@ -0,0 +1,361 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.filter;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.PrivateCellUtil;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.Durability;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.RegionInfoBuilder;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.testclassification.FilterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+/**
+ * Integration tests for HBASE-29974 Path 1: {@link
Filter#getHintForRejectedRow(Cell)} allows the
+ * scan pipeline to seek directly past rejected rows instead of iterating
through every cell in each
+ * rejected row one-by-one via {@code nextRow()}.
+ *
+ * Each test verifies two properties simultaneously:
+ *
+ * Correctness — the scan returns exactly the rows that are not
rejected by
+ * {@link Filter#filterRowKey(Cell)}, regardless of whether the hint path or
the legacy
+ * cell-iteration path is used.
+ * Efficiency — when a filter provides a hint that jumps over all N
rejected rows in one
+ * seek, {@code getHintForRejectedRow} is called exactly once (not N times),
proving that the
+ * scanner did not fall back to cell-by-cell iteration for the skipped
rows.
+ *
+ */
+@Category({ FilterTests.class, MediumTests.class })
+public class TestFilterHintForRejectedRow {
Review Comment:
Though we need both `getHintForRejectedRow()` and `getSkipHint()`
implementations,
for phoenix cdc, i believe `getSkipHint()` will more likely be common hint
than `getHintForRejectedRow()`, is that correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29974: Persist filter hints across scan circuit breaks [hbase]
virajjasani commented on code in PR #7882:
URL: https://github.com/apache/hbase/pull/7882#discussion_r3103755156
##
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestUserScanQueryMatcher.java:
##
@@ -398,6 +419,232 @@ scanWithFilter, new ScanInfo(this.conf, fam2, 0, 5, ttl,
KeepDeletedCells.FALSE,
assertArrayEquals(nextCell.getQualifierArray(), col4);
}
+ // ---
+ // Tests for HBASE-29974: Filter#getSkipHint consulted at matchColumn
+ // structural short-circuits (time-range, column, and version gates).
+ // ---
Review Comment:
Let's remove these lines
##
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterHintForRejectedRow.java:
##
@@ -0,0 +1,361 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.filter;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.PrivateCellUtil;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.Durability;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.RegionInfoBuilder;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.testclassification.FilterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+/**
+ * Integration tests for HBASE-29974 Path 1: {@link
Filter#getHintForRejectedRow(Cell)} allows the
+ * scan pipeline to seek directly past rejected rows instead of iterating
through every cell in each
+ * rejected row one-by-one via {@code nextRow()}.
+ *
+ * Each test verifies two properties simultaneously:
+ *
+ * Correctness — the scan returns exactly the rows that are not
rejected by
+ * {@link Filter#filterRowKey(Cell)}, regardless of whether the hint path or
the legacy
+ * cell-iteration path is used.
+ * Efficiency — when a filter provides a hint that jumps over all N
rejected rows in one
+ * seek, {@code getHintForRejectedRow} is called exactly once (not N times),
proving that the
+ * scanner did not fall back to cell-by-cell iteration for the skipped
rows.
+ *
+ */
+@Category({ FilterTests.class, MediumTests.class })
+public class TestFilterHintForRejectedRow {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestFilterHintForRejectedRow.class);
+
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+
+ private static final byte[] FAMILY = Bytes.toBytes("f");
+ private static final int CELLS_PER_ROW = 10;
+ private static final byte[] VALUE = Bytes.toBytes("value");
+
+ private HRegion region;
+
+ @Rule
+ public TestName name = new TestName();
+
+ @Before
+ public void setUp() throws Exception {
+TableDescriptor tableDescriptor =
+
TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName()))
+.setColumnFamily(ColumnFamilyDescriptorBuilder.of(FAMILY)).build();
+RegionInfo info =
RegionInfoBuilder.newBuilder(tableDescriptor.getTableName()).build();
+this.region = HBaseTestingUtil.createRegionAndWAL(info,
TEST_UTIL.getDataTestDir(),
+ TEST_UTIL.getCon
