Apache9 commented on PR #7389:
URL: https://github.com/apache/hbase/pull/7389#issuecomment-3419696409
> > Checked the code, this is only used in CompareFilter?
>
> Yes those are the only types of filters one can use the problematic byte
array comparator with.
>
> > Actually, all the filterXXX method in Filter can throw an IOException,
and I think usually throwing RuntimeException in Comparator implementation does
not hurt the whole system, so maybe a possible way is to catch RuntimeException
in Filter implemention, and convert it to a DoNotRetryIOException to indicate
that there is a misconfigured filter or code bug.
>
> Ah yes thank you - I see filter methods can throw IOException in the kinds
of cases such as this one - "Concrete implementers can signal a failure
condition in their code by throwing an {@link IOException}" - so the comparator
cannot throw IOException , but filter applying the comparator would be the next
best layer that can throw checked exception where its appropriate to do the
catching/wrapping of runtime exceptions coming from comparator below. I think
this is a much better generalized approach to handle issues at/below filter
layer.
>
> I think its safe to assume that if a runtime exception occurs during
filter application its extremely likely to happen again if the same scan RPC is
retried with the same filters/data. Do you think its appropriate to treat any
runtime exception that occurs during any filter application as
DoNotRetryIOException ? Or limit the runtime exception handling only to
`CompareFilter`? I think its best to handle all runtime exceptions for all
filters this way if it makes sense, in order to keep things consistent and
cover all possible code bugs/misconfigured filters/comparators that can lead to
runtime exception.
I think first we'd better only handle the offset out of bounds for
BinaryComponentComparator, so maybe we can still introduce a new type of
RuntimeException(maybe sub class of ArrayIndexOutOfBoundsIndex?), and in the
filter implementation we only catch this exception and convert it to a
DoNotRetryIOException, for others, maybe we can wrap it as a general
HBaseIOException to let client retry?
WDYT?
Thanks.
--
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]
