This is an automated email from the ASF dual-hosted git repository.

arina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 698ec5844d1769fa9d4a7c24380bfb7591c68805
Author: Paul Rogers <par0...@yahoo.com>
AuthorDate: Sun Oct 13 14:43:27 2019 -0700

    DRILL-7402: Suppress batch dumps for expected failures in tests
    
    Drill provides a way to dump the last few batches when an error
    occurs. However, in tests, we often deliberately cause something
    to fail. In this case, the batch dump is unnecessary.
    
    This enhancement adds a config property, disabled in tests, that
    controls the dump activity. The option is enabled in the one test
    that needs it enabled.
    
    closes #1872
---
 .../drill/exec/physical/impl/BaseRootExec.java     | 11 ++++++---
 .../java-exec/src/main/resources/drill-module.conf |  4 ++++
 .../java/org/apache/drill/TestOperatorDump.java    |  2 ++
 .../apache/drill/test/ClusterFixtureBuilder.java   |  2 +-
 .../java/org/apache/drill/test/ConfigBuilder.java  | 27 +++++++---------------
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java
index 95a1235..04ea6b6 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java
@@ -36,9 +36,10 @@ import org.apache.drill.exec.record.RecordBatch.IterOutcome;
 public abstract class BaseRootExec implements RootExec {
   private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(BaseRootExec.class);
 
-  protected OperatorStats stats = null;
-  protected OperatorContext oContext = null;
-  protected RootFragmentContext fragmentContext = null;
+  public static final String ENABLE_BATCH_DUMP_CONFIG = 
"drill.exec.debug.dump_batches";
+  protected OperatorStats stats;
+  protected OperatorContext oContext;
+  protected RootFragmentContext fragmentContext;
   private List<CloseableRecordBatch> operators;
 
   public BaseRootExec(final RootFragmentContext fragmentContext, final 
PhysicalOperator config) throws OutOfMemoryException {
@@ -113,6 +114,7 @@ public abstract class BaseRootExec implements RootExec {
       case OK:
         stats.batchReceived(0, b.getRecordCount(), false);
         break;
+      default:
     }
     return next;
   }
@@ -129,6 +131,9 @@ public abstract class BaseRootExec implements RootExec {
     if (operators == null) {
       return;
     }
+    if (!fragmentContext.getConfig().getBoolean(ENABLE_BATCH_DUMP_CONFIG)) {
+      return;
+    }
 
     final int numberOfBatchesToDump = 2;
     logger.error("Batch dump started: dumping last {} failed batches", 
numberOfBatchesToDump);
diff --git a/exec/java-exec/src/main/resources/drill-module.conf 
b/exec/java-exec/src/main/resources/drill-module.conf
index 62af353..7dfafd3 100644
--- a/exec/java-exec/src/main/resources/drill-module.conf
+++ b/exec/java-exec/src/main/resources/drill-module.conf
@@ -272,6 +272,10 @@ drill.exec: {
     // the command line:
     // java ... -ea -Ddrill.exec.debug.validate_vectors=true ...
     validate_vectors: false
+    // If true, dumps several record batches when an operator fails.
+    // Generally disabled in testing, especially when we expect
+    // a failure
+    dump_batches: true
   },
   spill: {
     // *** Options common to all the operators that may spill
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/TestOperatorDump.java 
b/exec/java-exec/src/test/java/org/apache/drill/TestOperatorDump.java
index 18ba61c..2c0fc99 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestOperatorDump.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestOperatorDump.java
@@ -21,6 +21,7 @@ import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.ConsoleAppender;
 import org.apache.drill.common.exceptions.UserRemoteException;
 import org.apache.drill.exec.exception.OutOfMemoryException;
+import org.apache.drill.exec.physical.impl.BaseRootExec;
 import org.apache.drill.exec.physical.impl.ScanBatch;
 import org.apache.drill.exec.physical.impl.xsort.managed.ExternalSortBatch;
 import org.apache.drill.exec.testing.Controls;
@@ -64,6 +65,7 @@ public class TestOperatorDump extends ClusterTest {
     logFixture = LogFixture.builder()
         .toConsole(appender, LogFixture.DEFAULT_CONSOLE_FORMAT)
         .build();
+    builder.configBuilder().put(BaseRootExec.ENABLE_BATCH_DUMP_CONFIG, "true");
     startCluster(builder);
   }
 
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixtureBuilder.java 
b/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixtureBuilder.java
index fb2813f..9657a5e 100644
--- 
a/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixtureBuilder.java
+++ 
b/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixtureBuilder.java
@@ -21,10 +21,10 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.ZookeeperHelper;
 import org.apache.drill.exec.server.options.OptionDefinition;
+import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
 
 /**
  * Build a Drillbit and client with the options provided. The simplest
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java 
b/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
index 9f26b2e..5b38182 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/ConfigBuilder.java
@@ -18,19 +18,20 @@
 package org.apache.drill.test;
 
 import java.util.Collection;
-import java.util.Properties;
 import java.util.Map.Entry;
+import java.util.Properties;
 
-import com.typesafe.config.ConfigValue;
 import org.apache.drill.common.config.DrillConfig;
-
-import com.typesafe.config.Config;
-import com.typesafe.config.ConfigValueFactory;
 import org.apache.drill.common.map.CaseInsensitiveMap;
 import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.physical.impl.BaseRootExec;
 import org.apache.drill.exec.server.options.OptionDefinition;
 import org.apache.drill.exec.server.options.SystemOptionManager;
 
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigValue;
+import com.typesafe.config.ConfigValueFactory;
+
 /**
  * Builds a {@link DrillConfig} for use in tests. Use this when a config
  * is needed by itself, separate from an embedded Drillbit.
@@ -38,7 +39,7 @@ import 
org.apache.drill.exec.server.options.SystemOptionManager;
 public class ConfigBuilder {
 
   protected String configResource;
-  protected Properties configProps = new Properties();
+  protected Properties configProps = createDefaultProperties();
   protected CaseInsensitiveMap<OptionDefinition> definitions = 
SystemOptionManager.createDefaultOptionDefinitions();
 
   /**
@@ -53,12 +54,7 @@ public class ConfigBuilder {
       throw new IllegalArgumentException( "Cannot provide both a config 
resource and config properties.");
     }
 
-    if (this.configProps == null) {
-      this.configProps = createDefaultProperties();
-    }
-
     this.configProps.putAll(configProps);
-
     return this;
   }
 
@@ -68,10 +64,6 @@ public class ConfigBuilder {
       throw new IllegalArgumentException( "Cannot provide both a config 
resource and config properties.");
     }
 
-    if (configProps == null) {
-      configProps = createDefaultProperties();
-    }
-
     for (Entry<String, ConfigValue> entry: drillConfig.entrySet()) {
       final Object key = entry.getKey();
       final Object value = entry.getValue().unwrapped();
@@ -126,10 +118,6 @@ public class ConfigBuilder {
       throw new IllegalArgumentException( "Cannot provide both a config 
resource and config properties.");
     }
 
-    if (configProps == null) {
-      configProps = createDefaultProperties();
-    }
-
     if (value instanceof Collection) {
       configProps.put(key, value);
     } else {
@@ -145,6 +133,7 @@ public class ConfigBuilder {
     properties.put(ExecConstants.CAST_EMPTY_STRING_TO_NULL, "false");
     properties.put(ExecConstants.USE_DYNAMIC_UDFS_KEY, "false");
     properties.put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, 
"false");
+    properties.put(BaseRootExec.ENABLE_BATCH_DUMP_CONFIG, "false");
 
     return properties;
   }

Reply via email to