DRILL-1181: Generate user friendly error messages
Conflicts:
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/4ac898c5
Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/4ac898c5
Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/4ac898c5
Branch: refs/heads/master
Commit: 4ac898c578d724b156cf4773644547b993407487
Parents: 6d48ff8
Author: Hanifi Gunes <hgunes@maprtech.com>
Authored: Wed Jul 23 18:29:59 2014 -0700
Committer: Aditya Kishore <aditya@maprtech.com>
Committed: Thu Jul 24 16:17:06 2014 -0700
----------------------------------------------------------------------
.../org/apache/drill/exec/ExecConstants.java | 3 ++
.../org/apache/drill/exec/ops/QueryContext.java | 1 +
.../drill/exec/physical/impl/ScreenCreator.java | 11 +++--
.../BroadcastSenderRootExec.java | 5 +-
.../impl/partitionsender/StatusHandler.java | 5 +-
.../drill/exec/rpc/user/QueryResultHandler.java | 18 +++++--
.../server/options/SystemOptionManager.java | 5 +-
.../org/apache/drill/exec/work/ErrorHelper.java | 51 +++++++++++++-------
.../apache/drill/exec/work/foreman/Foreman.java | 5 +-
.../work/fragment/AbstractStatusReporter.java | 4 +-
.../org/apache/drill/jdbc/DrillResultSet.java | 2 +-
11 files changed, 80 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index 15c3b88..595fdd7 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -131,5 +131,8 @@ public interface ExecConstants {
public static final String QUEUE_TIMEOUT_KEY = "exec.queue.timeout_millis";
public static final OptionValidator QUEUE_TIMEOUT = new PositiveLongValidator(QUEUE_TIMEOUT_KEY,
Long.MAX_VALUE, 60*1000*5);
+ public static final String ENABLE_VERBOSE_ERRORS_KEY = "exec.errors.verbose";
+ public static final OptionValidator ENABLE_VERBOSE_ERRORS = new BooleanValidator(ENABLE_VERBOSE_ERRORS_KEY,
false);
+
public static final String BOOTSTRAP_STORAGE_PLUGINS_FILE = "bootstrap-storage-plugins.json";
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
index be3a3ef..e6c6fa7 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
@@ -92,6 +92,7 @@ public class QueryContext{
public OptionManager getOptions(){
return session.getOptions();
}
+
public DrillbitEndpoint getCurrentEndpoint(){
return drillbitContext.getEndpoint();
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java
index 853969d..279374f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java
@@ -22,6 +22,7 @@ import io.netty.buffer.ByteBuf;
import java.util.List;
import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.memory.OutOfMemoryException;
import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.ops.MetricDef;
@@ -99,11 +100,13 @@ public class ScreenCreator implements RootCreator<Screen>{
switch(outcome){
case STOP: {
sendCount.waitForSendComplete();
- QueryResult header = QueryResult.newBuilder() //
+ boolean verbose = context.getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ QueryResult header = QueryResult.newBuilder() //
.setQueryId(context.getHandle().getQueryId()) //
.setRowCount(0) //
.setQueryState(QueryState.FAILED)
- .addError(ErrorHelper.logAndConvertError(context.getIdentity(), "Screen received
stop request sent.", context.getFailureCause(), logger))
+ .addError(ErrorHelper.logAndConvertError(context.getIdentity(), "Screen received
stop request sent.",
+ context.getFailureCause(), logger, verbose))
.setDef(RecordBatchDef.getDefaultInstance()) //
.setIsLastChunk(true) //
.build();
@@ -193,7 +196,9 @@ public class ScreenCreator implements RootCreator<Screen>{
public void failed(RpcException ex) {
sendCount.decrement();
logger.error("Failure while sending data to user.", ex);
- ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger);
+ boolean verbose = context.getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger,
+ verbose);
ok = false;
this.ex = ex;
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
index bdaf8ca..36e54f9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
@@ -21,6 +21,7 @@ import io.netty.buffer.ByteBuf;
import java.util.List;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.memory.OutOfMemoryException;
import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.ops.MetricDef;
@@ -185,7 +186,9 @@ public class BroadcastSenderRootExec extends BaseRootExec {
public void failed(RpcException ex) {
sendCount.decrement();
logger.error("Failure while sending data to user.", ex);
- ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger);
+ boolean verbose = context.getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger,
+ verbose);
ok = false;
this.ex = ex;
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java
index e3f9eae..7c3fd52 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java
@@ -19,6 +19,7 @@ package org.apache.drill.exec.physical.impl.partitionsender;
import io.netty.buffer.ByteBuf;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.physical.impl.SendingAccountor;
import org.apache.drill.exec.proto.GeneralRPCProtos.Ack;
@@ -48,7 +49,9 @@ public class StatusHandler extends BaseRpcOutcomeListener<Ack> {
public void failed(RpcException ex) {
sendCount.decrement();
logger.error("Failure while sending data to user.", ex);
- ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger);
+ boolean verbose = context.getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ ErrorHelper.logAndConvertError(context.getIdentity(), "Failure while sending fragment
to client.", ex, logger,
+ verbose);
ok = false;
this.ex = ex;
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
index e213c51..95b7446 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
@@ -22,9 +22,11 @@ import io.netty.buffer.ByteBuf;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
+import org.apache.drill.exec.proto.UserBitShared;
import org.apache.drill.exec.proto.UserBitShared.QueryId;
import org.apache.drill.exec.proto.UserBitShared.QueryResult;
import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.beans.DrillPBError;
import org.apache.drill.exec.rpc.BaseRpcOutcomeListener;
import org.apache.drill.exec.rpc.RpcBus;
import org.apache.drill.exec.rpc.RpcException;
@@ -70,12 +72,13 @@ public class QueryResultHandler {
}
}
- if(failed){
- l.submissionFailed(new RpcException("Remote failure while running query." + batch.getHeader().getErrorList()));
+ if(failed) {
+ String message = buildErrorMessage(batch);
+ l.submissionFailed(new RpcException(message));
resultsListener.remove(result.getQueryId(), l);
}else{
try {
- l.resultArrived(batch, throttle);
+ l.resultArrived(batch, throttle);
} catch (Exception e) {
batch.release();
l.submissionFailed(new RpcException(e));
@@ -91,6 +94,15 @@ public class QueryResultHandler {
}
}
+ protected String buildErrorMessage(QueryResultBatch batch) {
+ StringBuilder sb = new StringBuilder();
+ for (UserBitShared.DrillPBError error:batch.getHeader().getErrorList()) {
+ sb.append(error.getMessage());
+ sb.append("\n");
+ }
+ return sb.toString();
+ }
+
private void failAll() {
for (UserResultsListener l : resultsListener.values()) {
l.submissionFailed(new RpcException("Received result without QueryId"));
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
index 48f0df0..bb45264 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
@@ -68,11 +68,12 @@ public class SystemOptionManager implements OptionManager{
ExecConstants.LARGE_QUEUE_SIZE,
ExecConstants.QUEUE_THRESHOLD_SIZE,
ExecConstants.QUEUE_TIMEOUT,
- ExecConstants.SMALL_QUEUE_SIZE,
+ ExecConstants.SMALL_QUEUE_SIZE,
ExecConstants.MIN_HASH_TABLE_SIZE,
ExecConstants.MAX_HASH_TABLE_SIZE,
QueryClassLoader.JAVA_COMPILER_VALIDATOR,
- QueryClassLoader.JAVA_COMPILER_JANINO_MAXSIZE
+ QueryClassLoader.JAVA_COMPILER_JANINO_MAXSIZE,
+ ExecConstants.ENABLE_VERBOSE_ERRORS
};
public final PStoreConfig<OptionValue> config;
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java
index a787273..7bb2919 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java
@@ -17,8 +17,11 @@
*/
package org.apache.drill.exec.work;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.util.UUID;
+import org.apache.drill.common.exceptions.DrillException;
import org.apache.drill.exec.planner.sql.parser.impl.ParseException;
import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
@@ -27,39 +30,53 @@ import org.slf4j.Logger;
public class ErrorHelper {
-
- public static DrillPBError logAndConvertError(DrillbitEndpoint endpoint, String message,
Throwable t, Logger logger){
+
+// public static DrillPBError logAndConvertError(DrillbitEndpoint endpoint, String message,
Throwable t, Logger logger) {
+// return logAndConvertError(endpoint, message, t, logger, true);
+// }
+
+ public static DrillPBError logAndConvertError(DrillbitEndpoint endpoint, String message,
Throwable t, Logger logger,
+ boolean verbose){
String id = UUID.randomUUID().toString();
DrillPBError.Builder builder = DrillPBError.newBuilder();
builder.setEndpoint(endpoint);
builder.setErrorId(id);
StringBuilder sb = new StringBuilder();
- if(message != null){
- sb.append(message);
+ if (message != null) {
+ sb.append(message).append(" ");
+ }
+ sb.append("%s ").append("[").append(id).append("]");
+ if (verbose) {
+ sb.append("\n")
+ .append("Node details: ")
+ .append(endpoint.getAddress())
+ .append(":").append(endpoint.getControlPort())
+ .append("/").append(endpoint.getDataPort());
+ }
+
+ if (verbose) {
+ StringWriter errors = new StringWriter();
+ errors.write("\n");
+ t.printStackTrace(new PrintWriter(errors));
+ sb.append(errors);
}
+ Throwable original = t;
+ Throwable rootCause = null;
while (true) {
- sb.append(" < ");
- sb.append(t.getClass().getSimpleName());
- if(t.getMessage() != null){
- sb.append(":[ ");
- sb.append(t.getMessage());
- sb.append(" ]");
- }
+ rootCause = t;
if (t.getCause() == null || t.getCause() == t
- || (t instanceof SqlParseException && t.getCause() instanceof ParseException))
break;
+ || (t instanceof SqlParseException && t.getCause() instanceof ParseException))
break;
t = t.getCause();
}
-
- builder.setMessage(sb.toString());
-
+ String finalMsg = rootCause.getMessage() == null ? original.getMessage() : rootCause.getMessage();
+ builder.setMessage(String.format(sb.toString(), finalMsg));
builder.setErrorType(0);
// record the error to the log for later reference.
logger.error("Error {}: {}", id, message, t);
-
-
+
return builder.build();
}
}
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
index 9a67653..4cc3e63 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
@@ -69,6 +69,7 @@ import org.apache.drill.exec.rpc.RpcException;
import org.apache.drill.exec.rpc.RpcOutcomeListener;
import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.OptionManager;
import org.apache.drill.exec.util.AtomicState;
import org.apache.drill.exec.util.Pointer;
import org.apache.drill.exec.work.ErrorHelper;
@@ -157,7 +158,9 @@ public class Foreman implements Runnable, Closeable, Comparable<Object>{
if (!state.updateState(QueryState.PENDING, QueryState.FAILED))
logger.warn("Tried to update query state to FAILED, but was not RUNNING");
}
- DrillPBError error = ErrorHelper.logAndConvertError(context.getCurrentEndpoint(), message,
t, logger);
+
+ boolean verbose = getContext().getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ DrillPBError error = ErrorHelper.logAndConvertError(context.getCurrentEndpoint(), message,
t, logger, verbose);
QueryResult result = QueryResult //
.newBuilder() //
.addError(error) //
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
index 8d9da19..1983ebb 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java
@@ -17,6 +17,7 @@
*/
package org.apache.drill.exec.work.fragment;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.proto.BitControl.FragmentStatus;
import org.apache.drill.exec.proto.ExecProtos.FragmentHandle;
@@ -44,7 +45,8 @@ public abstract class AbstractStatusReporter implements StatusReporter{
context.getStats().addMetricsToStatus(b);
b.setState(state);
if(t != null){
- b.setError(ErrorHelper.logAndConvertError(context.getIdentity(), message, t, logger));
+ boolean verbose = context.getOptions().getOption(ExecConstants.ENABLE_VERBOSE_ERRORS_KEY).bool_val;
+ b.setError(ErrorHelper.logAndConvertError(context.getIdentity(), message, t, logger,
verbose));
}
status.setHandle(context.getHandle());
b.setMemoryUsed(context.getAllocator().getAllocatedMemory());
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4ac898c5/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
index 61fca61..bde0d3f 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java
@@ -123,7 +123,7 @@ public class DrillResultSet extends AvaticaResultSet {
this.ex = ex;
completed = true;
close();
- System.out.println("Query failed: " + ex);
+ System.out.println("Query failed: " + ex.getMessage());
}
@Override
|