HBASE-29672 Handle runtime comparison failures during filtering gracefully

[droudnitsky]

https://issues.apache.org/jira/browse/HBASE-29672

There is a large class of filters:

RowFilter
ValueFilter
QualifierFilter
FamilyFilter
DependentColumnFilter
ColumnValueFilter
SingleColumnValueFilter
SingleColumnValueExcludeFilter

Which take a ByteArrayComparable comparator as an argument (e.g BinaryComparator, RegexStringComparator, BinaryComponentComparator) and apply the given comparator at query runtime on the server. Due to filter misconfiguration/data shape/comparator bugs, a comparator may throw a runtime exception which filters are not currently handling. In this case the runtime exception gets propagated all the way up the call stack, leading to an unexpected throwable at the topmost RpcServer layer and a very unclear remote exception on the client with a very long mysterious server exception trace.

This PR adds runtime exception handling for comparator runtime exceptions and treats them as HBaseIOException, and propagate a clear exception message to the client. This approach will also lets us handle cases where we know that a client retry is guaranteed to fail and let us prevent bad requests from being excessively retried (such as HBASE-29654).

Some context/discussion in #7389

[droudnitsky]

Code snippet to identify all filters which take ByteArrayComparable, majority (5/8) filters are an extension of CompareFilter , but there are 3 outlier ColumnValue filters which do not extend CompareFilter -

 String pkg = "org.apache.hadoop.hbase.filter"; // <-- the package to scan
List<Class<?>> hits = new ArrayList<>();
try (ScanResult scan = new ClassGraph()
.acceptPackages(pkg)
.enableClassInfo()
.scan()) {
for (ClassInfo ci : scan.getAllClasses()) {
Class<?> cls = ci.loadClass();
for (Constructor<?> ctor : cls.getDeclaredConstructors()) {
for (Class<?> pt : ctor.getParameterTypes()) {
if (pt.getName().equals("org.apache.hadoop.hbase.filter.ByteArrayComparable") // FQCN safer
|| pt.getSimpleName().equals("ByteArrayComparable")) { // simple name
hits.add(cls);
break;
}
}
}
}
}
System.out.println("matches: " + hits.size());
org.apache.hadoop.hbase.filter.ColumnValueFilter
org.apache.hadoop.hbase.filter.CompareFilter
org.apache.hadoop.hbase.filter.DependentColumnFilter
org.apache.hadoop.hbase.filter.FamilyFilter
org.apache.hadoop.hbase.filter.QualifierFilter
org.apache.hadoop.hbase.filter.RowFilter
org.apache.hadoop.hbase.filter.SingleColumnValueExcludeFilter
org.apache.hadoop.hbase.filter.SingleColumnValueFilter

[droudnitsky]

Hey @Apache9 this PR is the followup from our discussion in here, if you get the chance could you please take a look, thank you

[Copilot]

Pull request overview

This PR adds graceful error handling for runtime exceptions that occur during comparator operations in HBase filters. When a ByteArrayComparable comparator throws a RuntimeException during filtering, it now gets wrapped in an HBaseIOException with a clear error message, preventing unclear remote exceptions on the client side.

Key Changes:

• Added runtime exception handling in CompareFilter's compare methods (compareRow, compareFamily, compareQualifier, compareValue)
• Updated method signatures to throw IOException for affected filter methods
• Added comprehensive unit tests to verify exception handling across all filter types

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
CompareFilter.java	Added wrapInHBaseIOException utility method and try-catch blocks to wrap RuntimeExceptions in all compare methods
ValueFilter.java	Updated filterCell method signature to throw IOException
SingleColumnValueFilter.java	Updated filterCell and filterColumnValue signatures and added exception wrapping
RowFilter.java	Updated filterRowKey method signature to throw IOException
QualifierFilter.java	Updated filterCell method signature to throw IOException
FamilyFilter.java	Updated filterCell method signature to throw IOException
DependentColumnFilter.java	Updated filterCell method signature to throw IOException
ColumnValueFilter.java	Updated private compareValue method signature and added exception wrapping
TestFiltersWithComparatorException.java	New comprehensive test suite with BadComparator to verify exception handling across all filter types

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Apache9]

Please fix the typo?

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Apache9]

@droudnitsky Not all the copilot comments need to be addressed...

Let me take a look again.

[Apache9]

Overall LGTM.

Will be good if we can migrate the test to use junit 5.

[Apache-HBase]

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 28s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ master Compile Tests _
+1 💚	mvninstall	3m 37s		master passed
+1 💚	compile	0m 21s		master passed
+1 💚	javadoc	0m 18s		master passed
+1 💚	shadedjars	6m 15s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 12s		the patch passed
+1 💚	compile	0m 21s		the patch passed
+1 💚	javac	0m 21s		the patch passed
+1 💚	javadoc	0m 17s		the patch passed
+1 💚	shadedjars	6m 13s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	1m 31s		hbase-client in the patch passed.
		23m 33s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7397/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7397
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 1f5dc8d5c2bb 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / 23f2311
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7397/6/testReport/
Max. process+thread count	287 (vs. ulimit of 30000)
modules	C: hbase-client U: hbase-client
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7397/6/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

[Apache-HBase]

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 28s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ master Compile Tests _
+1 💚	mvninstall	2m 58s		master passed
+1 💚	compile	0m 50s		master passed
+1 💚	checkstyle	0m 18s		master passed
+1 💚	spotbugs	0m 42s		master passed
+1 💚	spotless	0m 45s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+1 💚	mvninstall	2m 58s		the patch passed
+1 💚	compile	0m 48s		the patch passed
-0 ⚠️	javac	0m 48s	/results-compile-javac-hbase-client.txt	hbase-client generated 1 new + 127 unchanged - 0 fixed = 128 total (was 127)
+1 💚	blanks	0m 1s		The patch has no blanks issues.
+1 💚	checkstyle	0m 18s		the patch passed
+1 💚	spotbugs	0m 49s		the patch passed
+1 💚	hadoopcheck	11m 14s		Patch does not cause any errors with Hadoop 3.3.6 3.4.1.
+1 💚	spotless	0m 46s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 9s		The patch does not generate ASF License warnings.
		30m 19s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7397/6/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7397
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux fecd29a4cbaa 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / 23f2311
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	83 (vs. ulimit of 30000)
modules	C: hbase-client U: hbase-client
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7397/6/console
versions	git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

[droudnitsky]

Thank you very much for reviewing @Apache9 ! I have migrated the test to junit 5. I believe this patch is also good to be back-ported to 2.x , in terms of client compatibility we are still throwing IOException, let me know if you have any concerns with backporting, otherwise I can open PRs for 2.5/2.6/2.7

[Apache9]

Please open a PR against branch-2, usually the patch for branch-2 can be applied to other 2.x branches cleanly.

And please fix the error prone warning if possible.

Thanks.

[droudnitsky]

Thank you @Apache9 , I will open a PR for branch-2.

The error prone warning was generated for a class I did not modify, I can fix it, should that be done in seperate PR since its unrelated to this ?

[noslowerdna]

LGTM