drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdelhakim deneche" <adene...@gmail.com>
Subject Re: Review Request 32987: DRILL-2675: Implement a subset of User Exceptions to improve how errors are reported to the user
Date Wed, 15 Apr 2015 04:09:45 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/
-----------------------------------------------------------

(Updated April 15, 2015, 4:09 a.m.)


Review request for drill, Jacques Nadeau, Jason Altekruse, and Parth Chandra.


Changes
-------

- removed public methods from ErrorHelper.
- ErrorHelper and UserExceptionContext are package protected


Bugs: DRILL-2675
    https://issues.apache.org/jira/browse/DRILL-2675


Repository: drill-git


Description (updated)
-------

The general idea is: if a code wants to throw an exception that contains a meaningful error
message, it throws a UserException. The propagation code will make sure this exception is
propagated to the client. The user exception object doesn't contain the final error message,
but enough information about the error, the client will use this information to display a
better error message.
Any exception that is not a UserException (or one of it's subclasses) will be considered as
a system exception. For those exceptions the client will display the error id and drillbit
identity and the root exception's message.
Error objects sent to the client will still contain a stack trace that can be used to display
more information if the client has enabled the error verbose mode.

**List of changes**
- added UserException
- _RemoteUserException_ replaces _RemoteRpcException_
- moved _ErrorHelper_ from exec/work to common/exceptions
- Root executors don't log transmission errors anymore, they will be logged by the _FragmentContext_
- added 2 examples of how to use _UserException.Builder_
 . _DrillSqlWorker_ throws a parse error when it catches a _ValidationException_
 . _JsonRecordReader_ throws a data read error
- made changes to _DrillPBError_
 . enum _ErrorType_ error_type
- we make sure to wrap exceptions into _UserException_ before building a _DrillPBError_ in
_FragmentContext_, _FragmentExecutor_ and _Foreman_
- _AbstractStatusReporter_ doesn't log errors anymore, it just builds the corresponding _DrillPBError_
- _UserException_ has a context that can be used to add context information to existing user
errors. The context is treated as an array of strings that will be displayed along the error
message. The context object offers both add and push methods to add the context information
to the bottom or top of the context list
 . context automatically generates a unique error id that can be used to track errors in the
logs
 . context accepts _DrillbitEndpoint_ information that is added and displayed along the generated
message
- system errors will display the root exception message. This will allow Drill to provide
  a consistent experience even for exceptions not yet converted into proper user errors
- _UserException.Builder_ can be used to create user exceptions. It's the only way to generate
user/system exceptions
- _UserResultListener.submissionFailed()_ expects a _UserException_ instead of _RemoteRpcException_
 . fixed multiple unit tests that were expecting _RemoteRpcException_
 . improved unit tests that were extracting the exception class and message from the error
message itself
- added proper documentation for _UserException_, _UserException.Builder_, _UserExceptionContext_
and _ErrorHelper_
- _ErrorHelper_ and _UserExceptionContext_ are package protected classes


Diffs (updated)
-----

  common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/exceptions/UserException.java PRE-CREATION

  common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java PRE-CREATION

  common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java PRE-CREATION

  common/src/test/java/org/apache/drill/common/exceptions/TestUserException.java PRE-CREATION

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 579cf7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java 98948af

  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java da2229c 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java b98778d

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/CoordinationQueue.java 0016d6a 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteRpcException.java 14ea873 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java b974963 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java a1be83b

  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 934a094

  exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java fbbf0b8

  exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
cc7cb83 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java 0773d6c 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 23ef0d3 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 8626d5b

  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/SqlUnsupportedException.java
2299afa 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
1b0885d 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java a7e6c46

  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/StatusReporter.java 26b5d68

  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 0c2f0e5 
  exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java 5703bf9 
  exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 875fb25 
  exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java f62f060 
  exec/java-exec/src/test/java/org/apache/drill/TestStarQueries.java effef9b 
  exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 11d83f9 
  exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java e03098a

  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
55f0d75 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetPhysicalPlan.java
882cdbd 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java d666d06 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 24ef62b 
  protocol/src/main/java/org/apache/drill/exec/proto/SchemaUserBitShared.java f72d5e1 
  protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java 96a921b 
  protocol/src/main/java/org/apache/drill/exec/proto/beans/DrillPBError.java ac9cef5 
  protocol/src/main/protobuf/UserBitShared.proto 2938114 

Diff: https://reviews.apache.org/r/32987/diff/


Testing
-------

testing of latest patch in progress


Thanks,

abdelhakim deneche


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message