[
https://issues.apache.org/jira/browse/DRILL-7506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17006576#comment-17006576
]
ASF GitHub Bot commented on DRILL-7506:
---------------------------------------
paul-rogers commented on pull request #1948: DRILL-7506: Simplify code gen
error handling
URL: https://github.com/apache/drill/pull/1948
Revises error handling in code generation to push exceptions closer to the
source of the error to both provide clearer errors and simplify code.
## Background
Drill evolved to have 2 1/2 ways to report errors. This PR is a step towards
simplifying the execution engine to use just one form of error reporting.
The original design of the "Volcano iterator" envisioned a `STOP` status
that indicated that an error occurred. When an operator encountered and error,
it would:
* Log the error.
* Pass an exception describing the error to the fragment context, telling
the fragment context that the fragment has failed.
* Pass `STOP` up the iterator call stack to stop execution.
Different operators implemented this in different ways. Some performed
`close()` operations at the time the error was found. Others put themselves
into a special "stop" state. Some call `kill()` on incoming batches others do
not.
Even from the beginning, the fragment executor had to handle runtime
exceptions such as NPE, OOM and so on. As a result, some operators adopted a
second way to report errors: simply throw a runtime (unchecked) exception.
At the time that the managed sort was created, we put in effort to ensure
that the fragment executor properly closes all operators in a fragment when a
runtime exception was thrown. The years since have shown that this solution
works.
We also resolved many cases in which it was not clear when `close()` should
be called. When the exception is thrown (or `STOP`) is returned? In the
`kill()` call? Only by the fragment executor? It turned out that the only
reliable solution was for the fragment executor to call `close()`, from the
leaf-most to the root-most operators.
The "2 1/2th" solution is that code *should* throw a `UserException` that
contains information useful to the user to describe the error. Some operators
use `UserException`, others throw other unchecked exceptions.
This PR moves toward using `UserException` as the one reliable way to report
errors. This PR focuses on code generation. It pushes code gen error handling
close to the code gen itself to allow clearer error messages. Doing so avoids
the need to bubble code gen exceptions up the call stack, resulting in cleaner
operator code.
## Tests
All unit tests were rerun to ensure there is no behavior change from this
PR. A couple of tests were modified to account for the use of a standardized
`UserException` instead of the various other exceptions thrown previously.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
> Simplify code gen error handling
> --------------------------------
>
> Key: DRILL-7506
> URL: https://issues.apache.org/jira/browse/DRILL-7506
> Project: Apache Drill
> Issue Type: Improvement
> Affects Versions: 1.17.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
> Priority: Major
> Fix For: 1.18.0
>
>
> Code generation can generate a variety of errors. Most operators bubble these
> exceptions up several layers in the code before catching them. This patch
> moves error handling closer to the code gen itself to allow a) simpler code,
> and b) clearer error messages.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
