droudnitsky commented on code in PR #7389:
URL: https://github.com/apache/hbase/pull/7389#discussion_r2441072541
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -519,6 +520,7 @@ public Pair<Message, ExtendedCellScanner> call(RpcCall
call, MonitoredRPCHandler
// increment the number of requests that were exceptions.
metrics.exception(e);
+ if (e instanceof DoNotRetryRuntimeException) throw new
DoNotRetryIOException(e);
Review Comment:
Yes I see your point here, its not good to rely on runtime exception in the
general case of issues deep in the call stack. The issue with the comparator
that this is trying to address stems from the design of the comparator - I
think its unique in that as far as I'm aware its the only comparator which
breaks/cannot function if the shape of the data on the server clashes with the
parameter the user specified when they created the comparator.
Ideally , I think this should have been designed/implemented in a way that
allows the filter to skip rows/cells which are too short to be able to do the
comparison, so the filter can function normally regardless of the length of the
data on the server and skip where needed instead of erroring out, but because
it was implemented as a comparator, AFAIK we cannot do this kind of skipping,
the `compareTo` interface does not allow it, the only option is to error out if
its not possible to do the comparison in `compareTo`.
To your point this seems to be a unique problem that we do not want/need a
generalized solution for, given that in most cases `RuntimeException` is due to
a code bug and not due to a normal case where is a mismatch between the
parameter the user is providing and the data on the server. I am thinking to
remove the generic `DoNotRetryRuntimeException` that I added , and in
`RpcServer` only check for the specific `OffsetOutOfBoundsException` that only
this comparator throws, and wrap it in `DoNotRetryIOException`. What do you
think ? This way this special runtime exception handling only applies to this
specific case and nothing else.
--
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]
