drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacq...@apache.org
Subject [14/27] git commit: DRILL-1181: Generate user friendly error messages
Date Sun, 27 Jul 2014 18:47:00 GMT
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


Mime
View raw message