[
https://issues.apache.org/jira/browse/DRILL-5057?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul Rogers updated DRILL-5057:
-------------------------------
Description:
Drill operator implementations have a somewhat inconsistent way to handle
exceptions. Some are handled locally, others cause a kill/fail/stop sequence
(see notes below.) Others throw an (unchecked) {{UserException}} without any
clean-up until the {{close()}} method is called.
Some background: 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.
In other cases, the code does catch checked exceptions and performs local
clean-up. It seems that some times, local clean-up is needed, other times
global clean-up (via {{close()}} is sufficient. It is not clear which is which.
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.
Or, define a uniform exception handling strategy. (Translate all checked
exceptions to an un-checked {{UserException}}?)
was:
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.
> 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
>
> Drill operator implementations have a somewhat inconsistent way to handle
> exceptions. Some are handled locally, others cause a kill/fail/stop sequence
> (see notes below.) Others throw an (unchecked) {{UserException}} without any
> clean-up until the {{close()}} method is called.
> Some background: 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.
> In other cases, the code does catch checked exceptions and performs local
> clean-up. It seems that some times, local clean-up is needed, other times
> global clean-up (via {{close()}} is sufficient. It is not clear which is
> which.
> 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.
> Or, define a uniform exception handling strategy. (Translate all checked
> exceptions to an un-checked {{UserException}}?)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
