You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@drill.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2020/01/02 04:08:00 UTC

[jira] [Commented] (DRILL-7506) Simplify code gen error handling

    [ 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:
users@infra.apache.org


> 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)