You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by jn...@apache.org on 2017/06/03 04:46:01 UTC
[06/12] drill git commit: DRILL-5512: Standardize error handling in
ScanBatch
DRILL-5512: Standardize error handling in ScanBatch
Standardizes error handling to throw a UserException. Prior code threw
various exceptions, called the fail() method, or returned a variety of
status codes.
closes #838
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/78739889
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/78739889
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/78739889
Branch: refs/heads/master
Commit: 78739889164c8df84fee249310f6d72d1199ea04
Parents: 155820a
Author: Paul Rogers <pr...@maprtech.com>
Authored: Mon May 15 15:00:21 2017 -0700
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Fri Jun 2 21:43:14 2017 -0700
----------------------------------------------------------------------
.../drill/common/exceptions/UserException.java | 12 +++--
.../drill/exec/physical/impl/ScanBatch.java | 51 ++++++++++----------
.../apache/drill/common/types/TypeProtos.java | 4 +-
.../apache/drill/exec/proto/UserBitShared.java | 16 ++++--
protocol/src/main/protobuf/Types.proto | 7 +--
protocol/src/main/protobuf/UserBitShared.proto | 8 ++-
6 files changed, 56 insertions(+), 42 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
index 87b3fd4..dd4fd36 100644
--- a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
+++ b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java
@@ -77,6 +77,14 @@ public class UserException extends DrillRuntimeException {
* <p>The cause message will be used unless {@link Builder#message(String, Object...)} is called.
* <p>If the wrapped exception is, or wraps, a user exception it will be returned by {@link Builder#build(Logger)}
* instead of creating a new exception. Any added context will be added to the user exception as well.
+ * <p>
+ * This exception, previously deprecated, has been repurposed to indicate unspecified
+ * errors. In particular, the case in which a lower level bit of code throws an
+ * exception other than UserException. The catching code then only knows "something went
+ * wrong", but not enough information to categorize the error.
+ * <p>
+ * System errors also indicate illegal internal states, missing functionality, and other
+ * code-related errors -- all of which "should never occur."
*
* @see org.apache.drill.exec.proto.UserBitShared.DrillPBError.ErrorType#SYSTEM
*
@@ -84,10 +92,8 @@ public class UserException extends DrillRuntimeException {
* returned by the builder instead of creating a new user exception
* @return user exception builder
*
- * @deprecated This method should never need to be used explicitly, unless you are passing the exception to the
- * Rpc layer or UserResultListener.submitFailed()
*/
- @Deprecated
+
public static Builder systemError(final Throwable cause) {
return new Builder(DrillPBError.ErrorType.SYSTEM, cause);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
index 5a9af39..4218069 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
@@ -86,31 +86,32 @@ public class ScanBatch implements CloseableRecordBatch {
public ScanBatch(PhysicalOperator subScanConfig, FragmentContext context,
OperatorContext oContext, Iterator<RecordReader> readers,
- List<Map<String, String>> implicitColumns) throws ExecutionSetupException {
+ List<Map<String, String>> implicitColumns) {
this.context = context;
this.readers = readers;
if (!readers.hasNext()) {
- throw new ExecutionSetupException("A scan batch must contain at least one reader.");
+ throw UserException.systemError(
+ new ExecutionSetupException("A scan batch must contain at least one reader."))
+ .build(logger);
}
currentReader = readers.next();
this.oContext = oContext;
allocator = oContext.getAllocator();
mutator = new Mutator(oContext, allocator, container);
- boolean setup = false;
try {
oContext.getStats().startProcessing();
currentReader.setup(oContext, mutator);
- setup = true;
- } finally {
- // if we had an exception during setup, make sure to release existing data.
- if (!setup) {
- try {
- currentReader.close();
- } catch(final Exception e) {
- throw new ExecutionSetupException(e);
- }
+ } catch (ExecutionSetupException e) {
+ try {
+ currentReader.close();
+ } catch(final Exception e2) {
+ logger.error("Close failed for reader " + currentReader.getClass().getSimpleName(), e2);
}
+ throw UserException.systemError(e)
+ .addContext("Setup failed for", currentReader.getClass().getSimpleName())
+ .build(logger);
+ } finally {
oContext.getStats().stopProcessing();
}
this.implicitColumns = implicitColumns.iterator();
@@ -173,9 +174,8 @@ public class ScanBatch implements CloseableRecordBatch {
currentReader.allocate(mutator.fieldVectorMap());
} catch (OutOfMemoryException e) {
- logger.debug("Caught Out of Memory Exception", e);
clearFieldVectorMap();
- return IterOutcome.OUT_OF_MEMORY;
+ throw UserException.memoryError(e).build(logger);
}
while ((recordCount = currentReader.next()) == 0) {
try {
@@ -213,17 +213,16 @@ public class ScanBatch implements CloseableRecordBatch {
try {
currentReader.allocate(mutator.fieldVectorMap());
} catch (OutOfMemoryException e) {
- logger.debug("Caught OutOfMemoryException");
clearFieldVectorMap();
- return IterOutcome.OUT_OF_MEMORY;
+ throw UserException.memoryError(e).build(logger);
}
addImplicitVectors();
} catch (ExecutionSetupException e) {
- this.context.fail(e);
releaseAssets();
- return IterOutcome.STOP;
+ throw UserException.systemError(e).build(logger);
}
}
+
// At this point, the current reader has read 1 or more rows.
hasReadNonEmptyFile = true;
@@ -245,18 +244,15 @@ public class ScanBatch implements CloseableRecordBatch {
return IterOutcome.OK;
}
} catch (OutOfMemoryException ex) {
- context.fail(UserException.memoryError(ex).build(logger));
- return IterOutcome.STOP;
+ throw UserException.memoryError(ex).build(logger);
} catch (Exception ex) {
- logger.debug("Failed to read the batch. Stopping...", ex);
- context.fail(ex);
- return IterOutcome.STOP;
+ throw UserException.systemError(ex).build(logger);
} finally {
oContext.getStats().stopProcessing();
}
}
- private void addImplicitVectors() throws ExecutionSetupException {
+ private void addImplicitVectors() {
try {
if (implicitVectors != null) {
for (ValueVector v : implicitVectors.values()) {
@@ -274,7 +270,10 @@ public class ScanBatch implements CloseableRecordBatch {
}
}
} catch(SchemaChangeException e) {
- throw new ExecutionSetupException(e);
+ // No exception should be thrown here.
+ throw UserException.systemError(e)
+ .addContext("Failure while allocating implicit vectors")
+ .build(logger);
}
}
@@ -324,7 +323,7 @@ public class ScanBatch implements CloseableRecordBatch {
* this scan batch. Made visible so that tests can create this mutator
* without also needing a ScanBatch instance. (This class is really independent
* of the ScanBatch, but resides here for historical reasons. This is,
- * in turn, the only use of the genereated vector readers in the vector
+ * in turn, the only use of the generated vector readers in the vector
* package.)
*/
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java
----------------------------------------------------------------------
diff --git a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java
index ff5698a..1fa4848 100644
--- a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java
+++ b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java
@@ -170,7 +170,7 @@ public final class TypeProtos {
* <code>FLOAT4 = 18;</code>
*
* <pre>
- * 4 byte ieee 754
+ * 4 byte ieee 754
* </pre>
*/
FLOAT4(17, 18),
@@ -463,7 +463,7 @@ public final class TypeProtos {
* <code>FLOAT4 = 18;</code>
*
* <pre>
- * 4 byte ieee 754
+ * 4 byte ieee 754
* </pre>
*/
public static final int FLOAT4_VALUE = 18;
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java
----------------------------------------------------------------------
diff --git a/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java b/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java
index d28a13d..e4261df 100644
--- a/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java
+++ b/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java
@@ -2178,6 +2178,10 @@ public final class UserBitShared {
*
* <pre>
* equivalent to SQLNonTransientException.
+ * - unexpected internal state
+ * - uncategorized operation
+ * general user action is to contact the Drill team for
+ * assistance
* </pre>
*/
SYSTEM(8, 8),
@@ -2186,8 +2190,8 @@ public final class UserBitShared {
*
* <pre>
* equivalent to SQLFeatureNotSupportedException
- * - type change
- * - schema change
+ * - unimplemented feature, option, or execution path
+ * - schema change in operator that does not support it
* </pre>
*/
UNSUPPORTED_OPERATION(9, 9),
@@ -2286,6 +2290,10 @@ public final class UserBitShared {
*
* <pre>
* equivalent to SQLNonTransientException.
+ * - unexpected internal state
+ * - uncategorized operation
+ * general user action is to contact the Drill team for
+ * assistance
* </pre>
*/
public static final int SYSTEM_VALUE = 8;
@@ -2294,8 +2302,8 @@ public final class UserBitShared {
*
* <pre>
* equivalent to SQLFeatureNotSupportedException
- * - type change
- * - schema change
+ * - unimplemented feature, option, or execution path
+ * - schema change in operator that does not support it
* </pre>
*/
public static final int UNSUPPORTED_OPERATION_VALUE = 9;
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/protocol/src/main/protobuf/Types.proto
----------------------------------------------------------------------
diff --git a/protocol/src/main/protobuf/Types.proto b/protocol/src/main/protobuf/Types.proto
index 71fa4ac..b2b29f0 100644
--- a/protocol/src/main/protobuf/Types.proto
+++ b/protocol/src/main/protobuf/Types.proto
@@ -24,7 +24,7 @@ option optimize_for = SPEED;
enum MinorType {
LATE = 0; // late binding type
MAP = 1; // an empty map column. Useful for conceptual setup. Children listed within here
-
+
TINYINT = 3; // single byte signed integer
SMALLINT = 4; // two byte signed integer
INT = 5; // four byte signed integer
@@ -40,7 +40,7 @@ enum MinorType {
TIMESTAMPTZ = 15; // unix epoch time in millis
TIMESTAMP = 16; // TBD
INTERVAL = 17; // TBD
- FLOAT4 = 18; // 4 byte ieee 754
+ FLOAT4 = 18; // 4 byte ieee 754
FLOAT8 = 19; // 8 byte ieee 754
BIT = 20; // single bit value (boolean)
FIXEDCHAR = 21; // utf8 fixed length string, padded with spaces
@@ -77,11 +77,8 @@ message MajorType {
repeated MinorType sub_type = 7; // used by Union type
}
-
-
enum DataMode {
OPTIONAL = 0; // nullable
REQUIRED = 1; // non-nullable
REPEATED = 2; // single, repeated-field
}
-
http://git-wip-us.apache.org/repos/asf/drill/blob/78739889/protocol/src/main/protobuf/UserBitShared.proto
----------------------------------------------------------------------
diff --git a/protocol/src/main/protobuf/UserBitShared.proto b/protocol/src/main/protobuf/UserBitShared.proto
index b091711..65f9698 100644
--- a/protocol/src/main/protobuf/UserBitShared.proto
+++ b/protocol/src/main/protobuf/UserBitShared.proto
@@ -74,11 +74,15 @@ message DrillPBError{
*/
RESOURCE = 7;
/* equivalent to SQLNonTransientException.
+ * - unexpected internal state
+ * - uncategorized operation
+ * general user action is to contact the Drill team for
+ * assistance
*/
SYSTEM = 8;
/* equivalent to SQLFeatureNotSupportedException
- * - type change
- * - schema change
+ * - unimplemented feature, option, or execution path
+ * - schema change in operator that does not support it
*/
UNSUPPORTED_OPERATION = 9;
/* SQL validation exception