This is an automated email from the ASF dual-hosted git repository.
junegunn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 59fc59ca7c1 HBASE-27691 Prevent filters from seeing synthetic scan
start cells (#8485)
59fc59ca7c1 is described below
commit 59fc59ca7c10ab0cd9fa90a2c9afc70db3febee4
Author: Andrew Olson <[email protected]>
AuthorDate: Fri Aug 7 00:35:39 2026 -0500
HBASE-27691 Prevent filters from seeing synthetic scan start cells (#8485)
Co-authored-by: Andrew Olson <[email protected]>
Signed-off-by: Junegunn Choi <[email protected]>
---
.../hadoop/hbase/regionserver/StoreScanner.java | 7 +-
.../hadoop/hbase/regionserver/TestScanner.java | 189 +++++++++++++++++++++
.../hbase/regionserver/TestSeekOptimizations.java | 63 +++++--
3 files changed, 243 insertions(+), 16 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
index 86752f27a0f..1309c3456a8 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
@@ -279,8 +279,11 @@ public class StoreScanner extends
NonReversedNonLazyKeyValueScanner
// Always check bloom filter to optimize the top row seek for delete
// family marker.
- seekScanners(scanners, matcher.getStartKey(), explicitColumnQuery &&
lazySeekEnabledGlobally,
- parallelSeekEnabled);
+ // Filters must only see real Cells. A lazy seek can expose a synthetic
Cell
+ // for the scan start row, so disable it for non-Get filters.
+ boolean useLazySeek =
+ explicitColumnQuery && lazySeekEnabledGlobally && !(scan.hasFilter()
&& !scan.isGetScan());
+ seekScanners(scanners, matcher.getStartKey(), useLazySeek,
parallelSeekEnabled);
// set storeLimit
this.storeLimit = scan.getMaxResultsPerColumnFamily();
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
index a5078c12959..b2aa81c7103 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
@@ -29,8 +29,11 @@ import static org.junit.jupiter.api.Assertions.fail;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.NavigableSet;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.CompareOperator;
+import org.apache.hadoop.hbase.ExtendedCell;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HTestConst;
@@ -48,9 +51,12 @@ import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.filter.BinaryComparator;
+import org.apache.hadoop.hbase.filter.ByteArrayComparable;
import org.apache.hadoop.hbase.filter.Filter;
import org.apache.hadoop.hbase.filter.InclusiveStopFilter;
import org.apache.hadoop.hbase.filter.PrefixFilter;
+import org.apache.hadoop.hbase.filter.RowFilter;
import org.apache.hadoop.hbase.filter.WhileMatchFilter;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
@@ -95,6 +101,10 @@ public class TestScanner {
private static final long START_CODE = Long.MAX_VALUE;
+ private static final byte[] LAZY_SEEK_FAMILY = Bytes.toBytes("family");
+ private static final byte[] LAZY_SEEK_QUALIFIER = Bytes.toBytes("qualifier");
+ private static final byte[] LAZY_SEEK_ROW = Bytes.toBytes("row");
+
private HRegion region;
private byte[] firstRowBytes, secondRowBytes, thirdRowBytes;
@@ -113,6 +123,185 @@ public class TestScanner {
col1 = Bytes.toBytes("column1");
}
+ private static final class RecordingStoreScanner extends StoreScanner {
+ private boolean initialSeekWasLazy;
+
+ RecordingStoreScanner(HStore store, Scan scan, NavigableSet<byte[]>
columns)
+ throws IOException {
+ super(store, store.getScanInfo(), scan, columns, Long.MAX_VALUE);
+ }
+
+ @Override
+ protected void seekScanners(List<? extends KeyValueScanner> scanners,
ExtendedCell seekKey,
+ boolean isLazy, boolean isParallelSeek) throws IOException {
+ initialSeekWasLazy = isLazy;
+ super.seekScanners(scanners, seekKey, isLazy, isParallelSeek);
+ }
+ }
+
+ private static final class TrackingRowComparator extends ByteArrayComparable
{
+ private final List<byte[]> comparedRows = new ArrayList<>();
+
+ TrackingRowComparator(byte[] value) {
+ super(value);
+ }
+
+ @Override
+ public int compareTo(byte[] value, int offset, int length) {
+ comparedRows.add(Bytes.copy(value, offset, length));
+ return Bytes.compareTo(getValue(), 0, getValue().length, value, offset,
length);
+ }
+
+ @Override
+ public byte[] toByteArray() {
+ return getValue();
+ }
+ }
+
+ @Test
+ public void testFilterComparatorOnlySeesActualRows() throws Exception {
+ byte[] family = Bytes.toBytes("family");
+ byte[] qualifier = Bytes.toBytes("qualifier");
+ byte[] regionStartKey = new byte[] { 1 };
+ byte[] row = new byte[] { 1, 0, 1 };
+ TableDescriptor tableDescriptor =
+
TableDescriptorBuilder.newBuilder(TableName.valueOf("testFilterComparatorOnlySeesActualRows"))
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(family)
+ .setBloomFilterType(BloomType.ROWCOL).build())
+ .build();
+ TrackingRowComparator comparator = new TrackingRowComparator(row);
+
+ StoreScanner.enableLazySeekGlobally(true);
+ try {
+ this.region = TEST_UTIL.createLocalHRegion(tableDescriptor,
regionStartKey, null);
+ Put put = new Put(row);
+ put.addColumn(family, qualifier, Bytes.toBytes("value"));
+ region.put(put);
+ region.flush(true);
+
+ Scan scan = new Scan().withStartRow(regionStartKey);
+ scan.addColumn(family, qualifier);
+ scan.setFilter(new RowFilter(CompareOperator.EQUAL, comparator));
+ List<Cell> results = new ArrayList<>();
+ try (InternalScanner scanner = region.getScanner(scan)) {
+ assertFalse(scanner.next(results));
+ }
+
+ assertEquals(1, results.size());
+ assertTrue(CellUtil.matchingRows(results.get(0), row));
+ assertEquals(1, comparator.comparedRows.size());
+ assertTrue(Bytes.equals(row, comparator.comparedRows.get(0)));
+ } finally {
+
StoreScanner.enableLazySeekGlobally(StoreScanner.LAZY_SEEK_ENABLED_BY_DEFAULT);
+ HBaseTestingUtil.closeRegionAndWAL(this.region);
+ }
+ }
+
+ @Test
+ public void testWhileMatchFilterOnlySeesActualRows() throws Exception {
+ byte[] family = Bytes.toBytes("family");
+ byte[] qualifier = Bytes.toBytes("qualifier");
+ TableDescriptor tableDescriptor =
+
TableDescriptorBuilder.newBuilder(TableName.valueOf("testWhileMatchFilterOnlySeesActualRows"))
+
.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(family).build()).build();
+
+ StoreScanner.enableLazySeekGlobally(true);
+ try {
+ this.region = TEST_UTIL.createLocalHRegion(tableDescriptor, null, null);
+ List<String> rows = List.of("row1", "row2", "row3");
+ for (String row : rows) {
+ Put put = new Put(Bytes.toBytes(row)).addColumn(family, qualifier,
Bytes.toBytes("value"));
+ region.put(put);
+ }
+ region.flush(true);
+
+ Scan scan = new Scan().addColumn(family, qualifier)
+ .setFilter(new WhileMatchFilter(new
RowFilter(CompareOperator.NOT_EQUAL,
+ new BinaryComparator(HConstants.EMPTY_START_ROW))));
+ int scannedRows = 0;
+ try (InternalScanner scanner = region.getScanner(scan)) {
+ boolean hasMoreRows;
+ do {
+ List<Cell> results = new ArrayList<>();
+ hasMoreRows = scanner.next(results);
+ if (!results.isEmpty()) {
+ ++scannedRows;
+ }
+ } while (hasMoreRows);
+ }
+
+ assertEquals(rows.size(), scannedRows);
+ } finally {
+
StoreScanner.enableLazySeekGlobally(StoreScanner.LAZY_SEEK_ENABLED_BY_DEFAULT);
+ HBaseTestingUtil.closeRegionAndWAL(this.region);
+ }
+ }
+
+ @Test
+ public void testInitialLazySeekForUnfilteredExplicitColumnScan() throws
Exception {
+ Scan scan = new Scan().withStartRow(LAZY_SEEK_ROW);
+ scan.addColumn(LAZY_SEEK_FAMILY, LAZY_SEEK_QUALIFIER);
+ assertInitialLazySeek(scan, true, true);
+ }
+
+ @Test
+ public void testInitialLazySeekForFilteredGet() throws Exception {
+ Get get = new Get(LAZY_SEEK_ROW);
+ get.addColumn(LAZY_SEEK_FAMILY, LAZY_SEEK_QUALIFIER);
+ get.setFilter(new PrefixFilter(LAZY_SEEK_ROW));
+ assertInitialLazySeek(new Scan(get), true, true);
+ }
+
+ @Test
+ public void testInitialLazySeekForFilteredNonGetScan() throws Exception {
+ Scan scan = new Scan().withStartRow(LAZY_SEEK_ROW);
+ scan.addColumn(LAZY_SEEK_FAMILY, LAZY_SEEK_QUALIFIER);
+ scan.setFilter(new PrefixFilter(LAZY_SEEK_ROW));
+ assertInitialLazySeek(scan, true, false);
+ }
+
+ @Test
+ public void testInitialLazySeekForAllColumnScan() throws Exception {
+ assertInitialLazySeek(new Scan().withStartRow(LAZY_SEEK_ROW), true, false);
+ }
+
+ @Test
+ public void testInitialLazySeekWhenDisabledGlobally() throws Exception {
+ Scan scan = new Scan().withStartRow(LAZY_SEEK_ROW);
+ scan.addColumn(LAZY_SEEK_FAMILY, LAZY_SEEK_QUALIFIER);
+ assertInitialLazySeek(scan, false, false);
+ }
+
+ private void assertInitialLazySeek(Scan scan, boolean lazySeekEnabled,
boolean expected)
+ throws IOException {
+ StoreScanner.enableLazySeekGlobally(lazySeekEnabled);
+ try {
+ HStore store = createLazySeekTestStore();
+ try (RecordingStoreScanner scanner =
+ new RecordingStoreScanner(store, scan,
scan.getFamilyMap().get(LAZY_SEEK_FAMILY))) {
+ assertEquals(expected, scanner.initialSeekWasLazy);
+ }
+ } finally {
+
StoreScanner.enableLazySeekGlobally(StoreScanner.LAZY_SEEK_ENABLED_BY_DEFAULT);
+ if (this.region != null) {
+ HBaseTestingUtil.closeRegionAndWAL(this.region);
+ this.region = null;
+ }
+ }
+ }
+
+ private HStore createLazySeekTestStore() throws IOException {
+ TableDescriptor tableDescriptor = TableDescriptorBuilder
+ .newBuilder(TableName.valueOf("testInitialLazySeek"))
+
.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(LAZY_SEEK_FAMILY).build()).build();
+ this.region = TEST_UTIL.createLocalHRegion(tableDescriptor, null, null);
+ Put put = new Put(LAZY_SEEK_ROW);
+ put.addColumn(LAZY_SEEK_FAMILY, LAZY_SEEK_QUALIFIER,
Bytes.toBytes("value"));
+ region.put(put);
+ region.flush(true);
+ return region.getStore(LAZY_SEEK_FAMILY);
+ }
+
/**
* Test basic stop row filter works.
*/
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
index 3225f4a2b7f..8fc509ff4ef 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.regionserver;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
@@ -43,6 +44,7 @@ import
org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.filter.PrefixFilter;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
@@ -51,6 +53,7 @@ import org.apache.hadoop.hbase.util.Bytes;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.params.provider.Arguments;
import org.slf4j.Logger;
@@ -120,21 +123,19 @@ public class TestSeekOptimizations {
}
@BeforeEach
- public void setUp() {
+ public void setUp(TestInfo testInfo) throws IOException {
RNG.setSeed(91238123L);
expectedKVs.clear();
TEST_UTIL.getConfiguration().setInt(BloomFilterUtil.PREFIX_LENGTH_KEY, 10);
- }
- @TestTemplate
- public void testMultipleTimestampRanges() throws IOException {
// enable seek counting
StoreFileScanner.instrument();
ColumnFamilyDescriptor columnFamilyDescriptor =
ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes(FAMILY)).setCompressionType(comprAlgo)
.setBloomFilterType(bloomType).setMaxVersions(3).build();
- region = TEST_UTIL.createTestRegion("testMultipleTimestampRanges",
columnFamilyDescriptor);
+ region =
+ TEST_UTIL.createTestRegion(testInfo.getTestMethod().get().getName(),
columnFamilyDescriptor);
// Delete the given timestamp and everything before.
final long latestDelTS = USE_MANY_STORE_FILES ? 1397 : -1;
@@ -150,12 +151,15 @@ public class TestSeekOptimizations {
}
prepareExpectedKVs(latestDelTS);
+ }
+ @TestTemplate
+ public void testMultipleTimestampRanges() throws IOException {
for (int[] columnArr : COLUMN_SETS) {
for (int[] rowRange : ROW_RANGES) {
for (int maxVersions : MAX_VERSIONS_VALUES) {
for (boolean lazySeekEnabled : new boolean[] { false, true }) {
- testScan(columnArr, lazySeekEnabled, rowRange[0], rowRange[1],
maxVersions);
+ testScan(columnArr, lazySeekEnabled, rowRange[0], rowRange[1],
maxVersions, false);
}
}
}
@@ -176,8 +180,9 @@ public class TestSeekOptimizations {
+ String.format("%.2f%%", expectedSeekSavings * 100));
}
- private void testScan(final int[] columnArr, final boolean lazySeekEnabled,
final int startRow,
- final int endRow, int maxVersions) throws IOException {
+ private ScanResult testScan(final int[] columnArr, final boolean
lazySeekEnabled,
+ final int startRow, final int endRow, final int maxVersions, final boolean
filtered)
+ throws IOException {
StoreScanner.enableLazySeekGlobally(lazySeekEnabled);
final Scan scan = new Scan();
final Set<String> qualSet = new HashSet<>();
@@ -186,6 +191,9 @@ public class TestSeekOptimizations {
scan.addColumn(FAMILY_BYTES, Bytes.toBytes(qualStr));
qualSet.add(qualStr);
}
+ if (filtered) {
+ scan.setFilter(new PrefixFilter(Bytes.toBytes("row")));
+ }
scan.readVersions(maxVersions);
scan.withStartRow(rowBytes(startRow));
@@ -198,18 +206,23 @@ public class TestSeekOptimizations {
final long initialSeekCount = StoreFileScanner.getSeekCount();
final InternalScanner scanner = region.getScanner(scan);
+ final long scannerOpenSeekCount = StoreFileScanner.getSeekCount() -
initialSeekCount;
final List<Cell> results = new ArrayList<>();
final List<Cell> actualKVs = new ArrayList<>();
// Such a clumsy do-while loop appears to be the official way to use an
// internalScanner. scanner.next() return value refers to the _next_
// result, not to the one already returned in results.
- boolean hasNext;
- do {
- hasNext = scanner.next(results);
- actualKVs.addAll(results);
- results.clear();
- } while (hasNext);
+ try {
+ boolean hasNext;
+ do {
+ hasNext = scanner.next(results);
+ actualKVs.addAll(results);
+ results.clear();
+ } while (hasNext);
+ } finally {
+ scanner.close();
+ }
List<Cell> filteredKVs =
filterExpectedResults(qualSet, rowBytes(startRow), rowBytes(endRow),
maxVersions);
@@ -234,6 +247,7 @@ public class TestSeekOptimizations {
totalSeekDiligent += seekCount;
}
assertKVListsEqual(testDesc, filteredKVs, actualKVs);
+ return new ScanResult(actualKVs, scannerOpenSeekCount);
}
private List<Cell> filterExpectedResults(Set<String> qualSet, byte[]
startRow, byte[] endRow,
@@ -448,4 +462,25 @@ public class TestSeekOptimizations {
+ HBaseTestingUtil.safeGetAsStr(actual, i) + " (length " + aLen + ")"
+ additionalMsg);
}
}
+
+ @TestTemplate
+ public void testSeeksEagerlyWhenFiltered() throws IOException {
+ ScanResult filteredLazyResults = testScan(new int[] { 0 }, true, 0, 2, 1,
true);
+ ScanResult filteredEagerResults = testScan(new int[] { 0 }, false, 0, 2,
1, true);
+ assertKVListsEqual("Filtered explicit column scan results differ with lazy
seeking enabled",
+ filteredEagerResults.cells, filteredLazyResults.cells);
+ assertEquals(filteredEagerResults.scannerOpenSeekCount,
+ filteredLazyResults.scannerOpenSeekCount,
+ "Filtered explicit column scans must always eagerly seek");
+ }
+
+ private static final class ScanResult {
+ private final List<Cell> cells;
+ private final long scannerOpenSeekCount;
+
+ private ScanResult(List<Cell> cells, long scannerOpenSeekCount) {
+ this.cells = cells;
+ this.scannerOpenSeekCount = scannerOpenSeekCount;
+ }
+ }
}
