From 4d636ecddca027ae5edb58560607fd7617c5705d Mon Sep 17 00:00:00 2001 From: Daniel Roudnitsky Date: Thu, 10 Sep 2026 08:29:51 -0400 Subject: [PATCH 1/2] HBASE-29672 Handle runtime comparison failures during filtering gracefully Backport of HBASE-29672 to branch-2. Filters that accept a user-supplied ByteArrayComparable now wrap any RuntimeException thrown while applying the comparator in an HBaseIOException, so it surfaces as a clear remote exception instead of escaping as an unexpected throwable at the RpcServer layer. branch-2 differences from the master patch: * CompareFilter retains the deprecated CompareOp overloads of compareRow/compareFamily/compareQualifier/compareValue. These are left untouched; only the CompareOperator variants wrap comparator failures. * The deprecated filterKeyValue(Cell) delegates in FamilyFilter, QualifierFilter, ValueFilter, DependentColumnFilter and SingleColumnValueFilter now declare throws IOException, since the filterCell(Cell) they delegate to does. Filter.filterKeyValue(Cell) already declares it, so this is consistent with the base class. --- .../hbase/filter/ColumnValueFilter.java | 10 +- .../hadoop/hbase/filter/CompareFilter.java | 56 ++++- .../hbase/filter/DependentColumnFilter.java | 4 +- .../hadoop/hbase/filter/FamilyFilter.java | 4 +- .../hadoop/hbase/filter/QualifierFilter.java | 4 +- .../apache/hadoop/hbase/filter/RowFilter.java | 2 +- .../hbase/filter/SingleColumnValueFilter.java | 14 +- .../hadoop/hbase/filter/ValueFilter.java | 4 +- .../TestFiltersWithComparatorException.java | 205 ++++++++++++++++++ 9 files changed, 274 insertions(+), 29 deletions(-) create mode 100644 hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java index 1991100d0daa..0c0c0ed60be8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java @@ -121,12 +121,16 @@ public ReturnCode filterCell(Cell c) throws IOException { * @return true means cell should be filtered out, included otherwise. */ private boolean compareValue(final CompareOperator op, final ByteArrayComparable comparator, - final Cell cell) { + final Cell cell) throws IOException { if (op == CompareOperator.NO_OP) { return true; } - int compareResult = PrivateCellUtil.compareValue(cell, comparator); - return CompareFilter.compare(op, compareResult); + try { + int compareResult = PrivateCellUtil.compareValue(cell, comparator); + return CompareFilter.compare(op, compareResult); + } catch (RuntimeException e) { + throw CompareFilter.wrapInHBaseIOException(e, comparator); + } } /** diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java index 9ed9c526bb96..3219563e3c6e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java @@ -22,6 +22,7 @@ import java.util.Objects; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CompareOperator; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.PrivateCellUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.yetus.audience.InterfaceAudience; @@ -123,6 +124,21 @@ public boolean filterRowKey(Cell cell) throws IOException { return false; } + /** + * RuntimeException when applying a comparator indicates a code bug or misconfigured + * filter/comparator, we wrap it in `HBaseIOException` to provide a clear exception message/stack + * trace and prevent propagating a runtime exception up the call stack (which would lead to + * unexpected throwable at RpcServer layer and a complicated unclear remote exception on the + * client) + */ + public static HBaseIOException wrapInHBaseIOException(RuntimeException e, + ByteArrayComparable comparator) { + String msg = + String.format("Runtime exception occurred when applying comparator %s during filtering", + comparator.getClass().getSimpleName()); + return new HBaseIOException(msg, e); + } + /** * @deprecated Since 2.0.0. Will be removed in 3.0.0. Use * {@link #compareRow(CompareOperator, ByteArrayComparable, Cell)} @@ -138,12 +154,16 @@ protected boolean compareRow(final CompareOp compareOp, final ByteArrayComparabl } protected boolean compareRow(final CompareOperator op, final ByteArrayComparable comparator, - final Cell cell) { + final Cell cell) throws IOException { if (op == CompareOperator.NO_OP) { return true; } - int compareResult = PrivateCellUtil.compareRow(cell, comparator); - return compare(op, compareResult); + try { + int compareResult = PrivateCellUtil.compareRow(cell, comparator); + return compare(op, compareResult); + } catch (RuntimeException e) { + throw wrapInHBaseIOException(e, comparator); + } } /** @@ -161,12 +181,16 @@ protected boolean compareFamily(final CompareOp compareOp, final ByteArrayCompar } protected boolean compareFamily(final CompareOperator op, final ByteArrayComparable comparator, - final Cell cell) { + final Cell cell) throws IOException { if (op == CompareOperator.NO_OP) { return true; } - int compareResult = PrivateCellUtil.compareFamily(cell, comparator); - return compare(op, compareResult); + try { + int compareResult = PrivateCellUtil.compareFamily(cell, comparator); + return compare(op, compareResult); + } catch (RuntimeException e) { + throw wrapInHBaseIOException(e, comparator); + } } /** @@ -185,13 +209,17 @@ protected boolean compareQualifier(final CompareOp compareOp, } protected boolean compareQualifier(final CompareOperator op, final ByteArrayComparable comparator, - final Cell cell) { + final Cell cell) throws IOException { // We do not call through to the non-deprecated method for perf reasons. if (op == CompareOperator.NO_OP) { return true; } - int compareResult = PrivateCellUtil.compareQualifier(cell, comparator); - return compare(op, compareResult); + try { + int compareResult = PrivateCellUtil.compareQualifier(cell, comparator); + return compare(op, compareResult); + } catch (RuntimeException e) { + throw wrapInHBaseIOException(e, comparator); + } } /** @@ -210,12 +238,16 @@ protected boolean compareValue(final CompareOp compareOp, final ByteArrayCompara } protected boolean compareValue(final CompareOperator op, final ByteArrayComparable comparator, - final Cell cell) { + final Cell cell) throws IOException { if (op == CompareOperator.NO_OP) { return true; } - int compareResult = PrivateCellUtil.compareValue(cell, comparator); - return compare(op, compareResult); + try { + int compareResult = PrivateCellUtil.compareValue(cell, comparator); + return compare(op, compareResult); + } catch (RuntimeException e) { + throw wrapInHBaseIOException(e, comparator); + } } static boolean compare(final CompareOp op, int compareResult) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java index be59929dfc43..27d48c02a5c1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java @@ -138,12 +138,12 @@ public boolean filterAllRemaining() { @Deprecated @Override - public ReturnCode filterKeyValue(final Cell c) { + public ReturnCode filterKeyValue(final Cell c) throws IOException { return filterCell(c); } @Override - public ReturnCode filterCell(final Cell c) { + public ReturnCode filterCell(final Cell c) throws IOException { // Check if the column and qualifier match if (!CellUtil.matchingColumn(c, this.columnFamily, this.columnQualifier)) { // include non-matches for the time being, they'll be discarded afterwards diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FamilyFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FamilyFilter.java index a5c6c8d266a1..d2f0344732e8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FamilyFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FamilyFilter.java @@ -70,12 +70,12 @@ public FamilyFilter(final CompareOperator op, final ByteArrayComparable familyCo @Deprecated @Override - public ReturnCode filterKeyValue(final Cell c) { + public ReturnCode filterKeyValue(final Cell c) throws IOException { return filterCell(c); } @Override - public ReturnCode filterCell(final Cell c) { + public ReturnCode filterCell(final Cell c) throws IOException { int familyLength = c.getFamilyLength(); if (familyLength > 0) { if (compareFamily(getCompareOperator(), this.comparator, c)) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/QualifierFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/QualifierFilter.java index 328c7fd57c6c..6ca8c91cc699 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/QualifierFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/QualifierFilter.java @@ -67,12 +67,12 @@ public QualifierFilter(final CompareOperator op, final ByteArrayComparable quali @Deprecated @Override - public ReturnCode filterKeyValue(final Cell c) { + public ReturnCode filterKeyValue(final Cell c) throws IOException { return filterCell(c); } @Override - public ReturnCode filterCell(final Cell c) { + public ReturnCode filterCell(final Cell c) throws IOException { if (compareQualifier(getCompareOperator(), this.comparator, c)) { return ReturnCode.SKIP; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RowFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RowFilter.java index cdebf0a15dcc..c34de1b423bf 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RowFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RowFilter.java @@ -86,7 +86,7 @@ public ReturnCode filterCell(final Cell v) { } @Override - public boolean filterRowKey(Cell firstRowCell) { + public boolean filterRowKey(Cell firstRowCell) throws IOException { if (compareRow(getCompareOperator(), this.comparator, firstRowCell)) { this.filterOutRow = true; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java index 63233e772a15..17b048dc15be 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java @@ -214,12 +214,12 @@ public boolean filterRowKey(Cell cell) throws IOException { @Deprecated @Override - public ReturnCode filterKeyValue(final Cell c) { + public ReturnCode filterKeyValue(final Cell c) throws IOException { return filterCell(c); } @Override - public ReturnCode filterCell(final Cell c) { + public ReturnCode filterCell(final Cell c) throws IOException { // System.out.println("REMOVE KEY=" + keyValue.toString() + ", value=" + // Bytes.toString(keyValue.getValue())); if (this.matchedColumn) { @@ -240,9 +240,13 @@ public ReturnCode filterCell(final Cell c) { return ReturnCode.INCLUDE; } - private boolean filterColumnValue(final Cell cell) { - int compareResult = PrivateCellUtil.compareValue(cell, this.comparator); - return CompareFilter.compare(this.op, compareResult); + private boolean filterColumnValue(final Cell cell) throws IOException { + try { + int compareResult = PrivateCellUtil.compareValue(cell, this.comparator); + return CompareFilter.compare(this.op, compareResult); + } catch (RuntimeException e) { + throw CompareFilter.wrapInHBaseIOException(e, this.comparator); + } } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ValueFilter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ValueFilter.java index 5339530923fa..23b8c56f0121 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ValueFilter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ValueFilter.java @@ -67,12 +67,12 @@ public ValueFilter(final CompareOperator valueCompareOp, @Deprecated @Override - public ReturnCode filterKeyValue(final Cell c) { + public ReturnCode filterKeyValue(final Cell c) throws IOException { return filterCell(c); } @Override - public ReturnCode filterCell(final Cell c) { + public ReturnCode filterCell(final Cell c) throws IOException { if (compareValue(getCompareOperator(), this.comparator, c)) { return ReturnCode.SKIP; } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java new file mode 100644 index 000000000000..c6a88160f0f6 --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java @@ -0,0 +1,205 @@ +/* + * 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.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CompareOperator; +import org.apache.hadoop.hbase.HBaseIOException; +import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.testclassification.FilterTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag(SmallTests.TAG) +@Tag(FilterTests.TAG) +public class TestFiltersWithComparatorException { + + /** + * Tests that filters which take a ByteArrayComparable comparator handle runtime exceptions in the + * comparator layer, see HBASE-29672 + */ + + final byte[] cf = Bytes.toBytes("cf"); + final byte[] row = Bytes.toBytes("row1"); + final byte[] cq = Bytes.toBytes("q"); + final long ts = 12345L; + final byte[] value = Bytes.toBytes("value"); + final Cell testCell = new KeyValue(row, cf, cq, ts, value); + + @FunctionalInterface + interface FilterFunctionThrowable { + void run(Filter filter) throws IOException; + } + + // Every filterX method that Filter implements to test + final List filterFunctionsToTest = Arrays.asList((Filter filter) -> { + filter.filterRowKey(testCell); + }, (Filter filter) -> { + filter.filterAllRemaining(); + }, (Filter filter) -> { + filter.filterCell(testCell); + }, (Filter filter) -> { + filter.filterRowCells(new ArrayList<>(Collections.singletonList(testCell))); + }, (Filter filter) -> { + filter.filterRow(); + }); + + /** + * Comparator which throws RuntimeException for every `compareTo` method that + * `ByteArrayComparable` implements + keeps a counter for number of compareTo invocations / + * RuntimeExceptions thrown + */ + static class BadComparator extends ByteArrayComparable { + + public int compareToInvocations = 0; + + public BadComparator() { + super(new byte[1]); + } + + @Override + public byte[] toByteArray() { + return new byte[1]; + } + + @Override + public int compareTo(byte[] value, int offset, int length) { + compareToInvocations++; + throw new RuntimeException("comparator runtime exception"); + } + + @Override + public int compareTo(ByteBuffer value, int offset, int length) { + compareToInvocations++; + throw new RuntimeException("comparator runtime exception"); + } + + @Override + public int compareTo(byte[] value) { + compareToInvocations++; + throw new RuntimeException("comparator runtime exception"); + } + + } + + /** + * Verifies that a given {@link Filter} correctly triggers comparator logic and wraps any runtime + * exceptions happening at comparison time in {@link org.apache.hadoop.hbase.HBaseIOException} + *

+ * This method runs a set of predefined filter functions against the provided filter instance and + * checks that if a comparator invocation occurs which throws a RuntimeException, it gets wrapped + * in a {@code HBaseIOException} by the filter implementation The test fails if: + *

+ * @param filter the filter instance under test + * @param badComparator the comparator the filter was constructed with + **/ + private void testFilter(Filter filter, BadComparator badComparator) { + for (FilterFunctionThrowable filterFunction : filterFunctionsToTest) { + int invocationsBefore = badComparator.compareToInvocations; + boolean ioExceptionThrown = false; + try { + filterFunction.run(filter); + } catch (HBaseIOException e) { + ioExceptionThrown = true; + } catch (IOException ignored) { + } + if (invocationsBefore != badComparator.compareToInvocations) { + assertTrue(ioExceptionThrown, "IOException should have been thrown"); + } + } + if (badComparator.compareToInvocations == 0) { + fail(String.format("Filter %s never invoked the comparator for any of the functions tested - " + + "intended behavior was not tested, this is not expected", filter.getClass().getName())); + } + } + + @Test + public void testColumnValueFilter() { + BadComparator comparator = new BadComparator(); + ColumnValueFilter columnValueFilter = + new ColumnValueFilter(cf, cq, CompareOperator.EQUAL, comparator); + testFilter(columnValueFilter, comparator); + } + + @Test + public void testRowFilter() { + BadComparator comparator = new BadComparator(); + RowFilter rowFilter = new RowFilter(CompareOperator.EQUAL, comparator); + testFilter(rowFilter, comparator); + } + + @Test + public void testDependentColumnFilter() { + BadComparator comparator = new BadComparator(); + DependentColumnFilter filter = + new DependentColumnFilter(cf, cq, false, CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + + @Test + public void testFamilyFilter() { + BadComparator comparator = new BadComparator(); + FamilyFilter filter = new FamilyFilter(CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + + @Test + public void testQualifierFilter() { + BadComparator comparator = new BadComparator(); + QualifierFilter filter = new QualifierFilter(CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + + @Test + public void testSingleColumnValueExcludeFilter() { + BadComparator comparator = new BadComparator(); + SingleColumnValueExcludeFilter filter = + new SingleColumnValueExcludeFilter(cf, cq, CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + + @Test + public void testSingleColumnValueFilter() { + BadComparator comparator = new BadComparator(); + SingleColumnValueFilter filter = + new SingleColumnValueFilter(cf, cq, CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + + @Test + public void testValueFilter() { + BadComparator comparator = new BadComparator(); + ValueFilter filter = new ValueFilter(CompareOperator.EQUAL, comparator); + testFilter(filter, comparator); + } + +} From c1dcdb9443eb3fad16a62b46bd1acafb54333f6f Mon Sep 17 00:00:00 2001 From: Daniel Roudnitsky Date: Thu, 10 Sep 2026 09:18:58 -0400 Subject: [PATCH 2/2] HBASE-29672 Avoid checkstyle indentation violations in test lambda list --- .../TestFiltersWithComparatorException.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java index c6a88160f0f6..4e1f37ce24df 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithComparatorException.java @@ -58,17 +58,11 @@ interface FilterFunctionThrowable { } // Every filterX method that Filter implements to test - final List filterFunctionsToTest = Arrays.asList((Filter filter) -> { - filter.filterRowKey(testCell); - }, (Filter filter) -> { - filter.filterAllRemaining(); - }, (Filter filter) -> { - filter.filterCell(testCell); - }, (Filter filter) -> { - filter.filterRowCells(new ArrayList<>(Collections.singletonList(testCell))); - }, (Filter filter) -> { - filter.filterRow(); - }); + final List filterFunctionsToTest = + Arrays.asList(filter -> filter.filterRowKey(testCell), Filter::filterAllRemaining, + filter -> filter.filterCell(testCell), + filter -> filter.filterRowCells(new ArrayList<>(Collections.singletonList(testCell))), + Filter::filterRow); /** * Comparator which throws RuntimeException for every `compareTo` method that