I think you misunderstood me. I'm not talking about checked vs unchecked
here. Catching Throwable vs catching Exception is a different beast.
Specifically in the example I showed you:
try {
callback.doWithNode(nodes.get((index + i) % nodes.size()), this);
} catch (Throwable *thisThrowableIsAlwaysIgnored*) {
// retry the next one...
onFailure(e);
}
Also, this is in the client, which means you are hiding potentially
important exceptions from clients.
We found one similar catch throwable in the JDK, which meant that we did
not get any clue as to why our program did not work. We actual had to patch
the JDK to get the original error printed.
-
Henrik
On Monday, March 24, 2014 10:19:17 PM UTC+1, Jörg Prante wrote:
>
> I agree that programs that work in an environment with safe and
> predictable resource consumption should always follow an all-checked
> exception approach.
>
> In reality, ES threads that encounter OOM or suddenly fall short on
> resources like network connections for a few milliseconds can often
> survive, because there are so many threads that can temporarily exceed
> limits. In case of TransportClientNodesService, it is better to let ES
> retry for a certain amount, otherwise the failure would propagate to the
> user app, and ES JVM would exit with failure. Imagine a feeder app that
> would always exit in case of too large docs, or restarting nodes, instead
> of trying to continue...
>
> Jörg
>
>
> On Mon, Mar 24, 2014 at 9:08 PM, Henrik Nordvik
> <[email protected]<javascript:>
> > wrote:
>
>> Hi,
>>
>> A lot of places in ES are now catching Throwable. There are several
>> commits that have changed from catching Exception to catching Throwable.
>> For instance, TransportClientNodesService catches Throwable, ignores it
>> and retries the operation [1]. Some places it logs it as debug and
>> continues.
>>
>> Throwable includes OutOfMemoryError, StackOveflowError, LinkageError and
>> so on. These are usually unrecoverable problems, so catching Throwable is
>> usually not considered a good practice.
>>
>> I think this looks very odd. Is this intentional? Are you really sure you
>> want to continue when you get those errors thrown, no matter what?
>>
>>
>> "An Error is a subclass of Throwable that indicates serious problems that
>> a reasonable application should not try to catch."
>>
>>
>> -
>> Henrik
>>
>> [1]
>> https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java#L267
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "elasticsearch" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected] <javascript:>.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/elasticsearch/47ea9472-0606-4107-bc9c-a2dcd85c8d29%40googlegroups.com<https://groups.google.com/d/msgid/elasticsearch/47ea9472-0606-4107-bc9c-a2dcd85c8d29%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
--
You received this message because you are subscribed to the Google Groups
"elasticsearch" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/elasticsearch/8c704742-8fa0-4c98-a8c5-be2a3834917b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
