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; }