[
https://issues.apache.org/jira/browse/DRILL-5057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15687184#comment-15687184
]
Paul Rogers commented on DRILL-5057:
------------------------------------
Thanks! The meaning of UserException is a bit unclear, but here is one stab at
understanding it.
Sounds like UserException (think of it as "Exception to be sent to the User")
is the primary way to handle problems. It unwinds the {{next()}} call stack. It
does cleanup by calling {{close()}}.
The original external sort code works this way for errors caught in spilling.
So far, so good. But, the picture gets muddy. Consider external sort (ESB)
again. ESB handles errors in many different ways:
{code}
public IterOutcome innerNext() {
...
} catch (SchemaChangeException ex) {
// ESB forbids schema changes. Do kill/fail/stop
kill(false);
context.fail(UserException.unsupportedError(ex)
.message("Sort doesn't currently support sorts with changing
schemas").build(logger));
return IterOutcome.STOP;
} catch(ClassTransformationException | IOException ex) {
// Misc. bad stuff happened. Kill/fail/stop
kill(false);
context.fail(ex);
return IterOutcome.STOP;
} catch (UnsupportedOperationException e) {
// More bad stuff. But, just toss a generic runtime exception.
throw new RuntimeException(e);
}
// But note that UserException passes straight through
...
public BatchGroup mergeAndSpill(LinkedList<BatchGroup> batchGroups) throws
SchemaChangeException {
...
// But, here, we catch absolutely everything and translate to UserException
} catch (Throwable e) {
...
throw UserException.resourceError(e)
.message("External Sort encountered an error while spilling to disk")
.addContext(e.getMessage() /* more detail */)
.build(logger);
{code}
That is, we handle {{IOException}} by translating it to a {{UserException}}
when spilling, but let it bubble up the call stack to the above code other
times. Those other times we don't throw a {{UserException}}, we instead do the
fail/stop work ourselves.
And, if we get the {{UnsupportedOperationException}} we throw a
{{RuntimeException}}, not a {{UserException}}.
The net result is that exception handling is very confusing. When to translate
to {{UserException}}? When to do an explicit fail/stop? Why not handle
{{UserException}} the same way?
So, let's rephrase this request: consistent exception handling model for
operators.
Any suggestions from the veterans about the intended exception handling design?
What should be done in these cases?
> UserException is unchecked, but is the primary way to report errors
> -------------------------------------------------------------------
>
> Key: DRILL-5057
> URL: https://issues.apache.org/jira/browse/DRILL-5057
> Project: Apache Drill
> Issue Type: Improvement
> Affects Versions: 1.8.0
> Reporter: Paul Rogers
> Priority: Minor
>
> Java provides two flavors of exceptions: checked and unchecked. Checked
> exceptions must be declared by any method that can throw them:
> {code}
> public void foo( ) throws ACheckedException ...
> {code}
> Unchecked exceptions derive from Java's {{RuntimeException}} class and do not
> need such a declaration. Unchecked exceptions are supposed to handle
> program-error kinds of conditions: illegal states, array out of bounds -- the
> kind of thing that would clutter the code for errors that should never occur
> in working code. (See the [Java
> docs|https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html].)
> Drill's {{UserException}} class is the primary way to report that something
> went wrong. As a result, every operator should catch the exception and do
> necessary clean-up and termination. Yet, the exception is unchecked.
> To ensure proper clean-up, migrate {{UserException}} to be checked. To help
> the migration, perhaps define a new {{ExecutionException}} class that is
> checked, along with a new {{buildChecked}} method in the "builder". Then,
> over time, migrate all user exceptions to the new, checked version and ensure
> that proper cleanup is done.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
