Repository: tez Updated Branches: refs/heads/branch-0.7 fb8fbd594 -> 27b0f2fab
TEZ-2662. Provide a way to check whether AM or task opts are valid and error if not. (hitesh) Project: http://git-wip-us.apache.org/repos/asf/tez/repo Commit: http://git-wip-us.apache.org/repos/asf/tez/commit/27b0f2fa Tree: http://git-wip-us.apache.org/repos/asf/tez/tree/27b0f2fa Diff: http://git-wip-us.apache.org/repos/asf/tez/diff/27b0f2fa Branch: refs/heads/branch-0.7 Commit: 27b0f2fab69d888c9d3cc374bd56ee3e4a4504fe Parents: fb8fbd5 Author: Hitesh Shah <[email protected]> Authored: Fri Aug 28 10:56:34 2015 -0700 Committer: Hitesh Shah <[email protected]> Committed: Fri Aug 28 10:56:34 2015 -0700 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../java/org/apache/tez/client/TezClient.java | 31 +++++- .../org/apache/tez/client/TezClientUtils.java | 38 +++++-- .../org/apache/tez/common/JavaOptsChecker.java | 87 +++++++++++++++ .../main/java/org/apache/tez/dag/api/DAG.java | 17 ++- .../apache/tez/dag/api/TezConfiguration.java | 22 ++++ .../org/apache/tez/client/TestTezClient.java | 23 ++++ .../apache/tez/client/TestTezClientUtils.java | 62 ++++++++++- .../apache/tez/common/TestJavaOptsChecker.java | 111 +++++++++++++++++++ .../org/apache/tez/dag/api/TestDAGPlan.java | 33 ++++++ 10 files changed, 406 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 4bc5e7c..99850b1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -6,6 +6,7 @@ Release 0.7.1: Unreleased INCOMPATIBLE CHANGES ALL CHANGES: + TEZ-2662. Provide a way to check whether AM or task opts are valid and error if not. TEZ-2300. TezClient.stop() takes a lot of time or does not work sometimes TEZ-2734. Add a test to verify the filename generated by OnDiskMerge. TEZ-2732. DefaultSorter throws ArrayIndex exceptions on 2047 Mb size sort buffers http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/main/java/org/apache/tez/client/TezClient.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/client/TezClient.java b/tez-api/src/main/java/org/apache/tez/client/TezClient.java index 3a5302c..e89ae82 100644 --- a/tez-api/src/main/java/org/apache/tez/client/TezClient.java +++ b/tez-api/src/main/java/org/apache/tez/client/TezClient.java @@ -25,6 +25,7 @@ import java.util.Map; import javax.annotation.Nullable; +import org.apache.tez.common.JavaOptsChecker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.classification.InterfaceAudience.Private; @@ -115,6 +116,7 @@ public class TezClient { private Map<String, LocalResource> additionalLocalResources = Maps.newHashMap(); private TezApiVersionInfo apiVersionInfo; private HistoryACLPolicyManager historyACLPolicyManager; + private JavaOptsChecker javaOptsChecker = null; private int preWarmDAGCounter = 0; @@ -332,6 +334,28 @@ public class TezClient { } } + if (this.amConfig.getTezConfiguration().getBoolean( + TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_ENABLED, + TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_ENABLED_DEFAULT)) { + String javaOptsCheckerClassName = this.amConfig.getTezConfiguration().get( + TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_CLASS, ""); + if (!javaOptsCheckerClassName.isEmpty()) { + try { + javaOptsChecker = ReflectionUtils.createClazzInstance(javaOptsCheckerClassName); + } catch (Exception e) { + LOG.warn("Failed to initialize configured Java Opts Checker" + + " (" + TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_CLASS + + ") , checkerClass=" + javaOptsCheckerClassName + + ". Disabling checker.", e); + javaOptsChecker = null; + } + } else { + javaOptsChecker = new JavaOptsChecker(); + } + + } + + if (isSession) { LOG.info("Session mode. Starting session."); TezClientUtils.processTezLocalCredentialsFile(sessionCredentials, @@ -357,7 +381,7 @@ public class TezClient { sessionAppId, null, clientName, amConfig, tezJarResources, sessionCredentials, usingTezArchiveDeploy, apiVersionInfo, - historyACLPolicyManager); + historyACLPolicyManager, javaOptsChecker); // Set Tez Sessions to not retry on AM crashes if recovery is disabled if (!amConfig.getTezConfiguration().getBoolean( @@ -440,7 +464,7 @@ public class TezClient { Map<String, LocalResource> tezJarResources = getTezJarResources(sessionCredentials); DAGPlan dagPlan = TezClientUtils.prepareAndCreateDAGPlan(dag, amConfig, tezJarResources, - usingTezArchiveDeploy, sessionCredentials, aclConfigs); + usingTezArchiveDeploy, sessionCredentials, aclConfigs, javaOptsChecker); SubmitDAGRequestProto.Builder requestBuilder = SubmitDAGRequestProto.newBuilder(); requestBuilder.setDAGPlan(dagPlan).build(); @@ -810,7 +834,8 @@ public class TezClient { ApplicationSubmissionContext appContext = TezClientUtils .createApplicationSubmissionContext( appId, dag, dag.getName(), amConfig, tezJarResources, credentials, - usingTezArchiveDeploy, apiVersionInfo, historyACLPolicyManager); + usingTezArchiveDeploy, apiVersionInfo, historyACLPolicyManager, + javaOptsChecker); LOG.info("Submitting DAG to YARN" + ", applicationId=" + appId + ", dagName=" + dag.getName()); http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java b/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java index 26d5e2a..3223f97 100644 --- a/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java +++ b/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java @@ -39,6 +39,7 @@ import java.util.Map.Entry; import com.google.common.base.Strings; import org.apache.commons.lang.StringUtils; +import org.apache.tez.common.JavaOptsChecker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.classification.InterfaceAudience.Private; @@ -414,7 +415,8 @@ public class TezClientUtils { ApplicationId appId, DAG dag, String amName, AMConfiguration amConfig, Map<String, LocalResource> tezJarResources, Credentials sessionCreds, boolean tezLrsAsArchive, - TezApiVersionInfo apiVersionInfo, HistoryACLPolicyManager historyACLPolicyManager) + TezApiVersionInfo apiVersionInfo, HistoryACLPolicyManager historyACLPolicyManager, + JavaOptsChecker javaOptsChecker) throws IOException, YarnException { Preconditions.checkNotNull(sessionCreds); @@ -604,7 +606,7 @@ public class TezClientUtils { if(dag != null) { DAGPlan dagPB = prepareAndCreateDAGPlan(dag, amConfig, tezJarResources, tezLrsAsArchive, - sessionCreds); + sessionCreds, null, javaOptsChecker); // emit protobuf DAG file style Path binaryPath = TezCommonUtils.getTezBinPlanStagingPath(tezSysStagingPath); @@ -680,16 +682,17 @@ public class TezClientUtils { Map<String, LocalResource> tezJarResources, boolean tezLrsAsArchive, Credentials credentials) throws IOException { return prepareAndCreateDAGPlan(dag, amConfig, tezJarResources, tezLrsAsArchive, credentials, - null); + null, null); } static DAGPlan prepareAndCreateDAGPlan(DAG dag, AMConfiguration amConfig, Map<String, LocalResource> tezJarResources, boolean tezLrsAsArchive, - Credentials credentials, Map<String, String> additionalDAGConfigs) throws IOException { + Credentials credentials, Map<String, String> additionalDAGConfigs, + JavaOptsChecker javaOptsChecker) throws IOException { Credentials dagCredentials = setupDAGCredentials(dag, credentials, amConfig.getTezConfiguration()); return dag.createDag(amConfig.getTezConfiguration(), dagCredentials, tezJarResources, - amConfig.getBinaryConfLR(), tezLrsAsArchive, additionalDAGConfigs); + amConfig.getBinaryConfLR(), tezLrsAsArchive, additionalDAGConfigs, javaOptsChecker); } static void maybeAddDefaultLoggingJavaOpts(String logLevel, List<String> vargs) { @@ -718,21 +721,39 @@ public class TezClientUtils { } return StringUtils.join(vargs, " ").trim(); } - + + @Private + public static String addDefaultsToTaskLaunchCmdOpts(String vOpts, Configuration conf) + throws TezException { + return addDefaultsToTaskLaunchCmdOpts(vOpts, conf, null); + } + @Private - public static String addDefaultsToTaskLaunchCmdOpts(String vOpts, Configuration conf) { + public static String addDefaultsToTaskLaunchCmdOpts(String vOpts, Configuration conf, + JavaOptsChecker javaOptsChecker) throws TezException { String vConfigOpts = ""; String taskDefaultOpts = conf.get(TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS, TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS_DEFAULT); if (taskDefaultOpts != null && !taskDefaultOpts.isEmpty()) { vConfigOpts = taskDefaultOpts + " "; } + String defaultTaskCmdOpts = TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT; + if (vOpts != null && !vOpts.isEmpty()) { + // Only use defaults if nothing is specified by the user + defaultTaskCmdOpts = ""; + } + vConfigOpts = vConfigOpts + conf.get(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, - TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT); + defaultTaskCmdOpts); if (vConfigOpts != null && !vConfigOpts.isEmpty()) { // Add options specified in the DAG at the end. vOpts = vConfigOpts + " " + vOpts; } + + if (javaOptsChecker != null) { + javaOptsChecker.checkOpts(vOpts); + } + return vOpts; } @@ -976,6 +997,7 @@ public class TezClientUtils { amOpts = maybeAddDefaultMemoryJavaOpts(amOpts, capability, tezConf.getDouble(TezConfiguration.TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION, TezConfiguration.TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION_DEFAULT)); + return amOpts; } http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/main/java/org/apache/tez/common/JavaOptsChecker.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/common/JavaOptsChecker.java b/tez-api/src/main/java/org/apache/tez/common/JavaOptsChecker.java new file mode 100644 index 0000000..7e7c231 --- /dev/null +++ b/tez-api/src/main/java/org/apache/tez/common/JavaOptsChecker.java @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tez.common; + +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.hadoop.classification.InterfaceAudience.Private; +import org.apache.hadoop.classification.InterfaceStability.Unstable; +import org.apache.tez.dag.api.TezException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Unstable +@Private +public class JavaOptsChecker { + + private static final Logger LOG = LoggerFactory.getLogger(JavaOptsChecker.class); + private static final Pattern pattern = Pattern.compile("\\s*(-XX:([\\+|\\-]?)(\\S+))\\s*"); + + public void checkOpts(String opts) throws TezException { + Set<String> gcOpts = new TreeSet<String>(); + if (LOG.isDebugEnabled()) { + LOG.debug("Checking JVM GC opts: " + opts); + } + Matcher matcher = pattern.matcher(opts); + while (matcher.find()) { + if (matcher.groupCount() != 3) { + continue; + } + + String opt = matcher.group(3); + if (!opt.endsWith("GC")) { + continue; + } + + int val = ( matcher.group(2).equals("+") ? 1 : -1 ); + if (gcOpts.contains(opt)) { + val += 1; + } + + if (val > 0) { + gcOpts.add(opt); + } else { + gcOpts.remove(opt); + } + } + + if (gcOpts.size() > 1) { + // Handle special case for " -XX:+UseParNewGC -XX:+UseConcMarkSweepGC " + // which can be specified together. + if (gcOpts.size() == 2) { + if (gcOpts.contains("UseParNewGC") + && gcOpts.contains("UseConcMarkSweepGC")) { + return; + } + } + + if (LOG.isDebugEnabled()) { + LOG.debug("Found clashing GC opts" + + ", conflicting GC Values=" + gcOpts); + } + throw new TezException("Invalid/conflicting GC options found," + + " cmdOpts=\"" + opts + "\""); + } + + } + +} http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java b/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java index 8ee1682..122a1b6 100644 --- a/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java +++ b/tez-api/src/main/java/org/apache/tez/dag/api/DAG.java @@ -32,6 +32,7 @@ import java.util.Stack; import org.apache.commons.collections4.BidiMap; import org.apache.commons.collections4.bidimap.DualLinkedHashBidiMap; +import org.apache.tez.common.JavaOptsChecker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.classification.InterfaceAudience.Private; @@ -692,14 +693,15 @@ public class DAG { Map<String, LocalResource> tezJarResources, LocalResource binaryConfig, boolean tezLrsAsArchive) { return createDag(tezConf, extraCredentials, tezJarResources, binaryConfig, tezLrsAsArchive, - null); + null, null); } // create protobuf message describing DAG @Private public synchronized DAGPlan createDag(Configuration tezConf, Credentials extraCredentials, Map<String, LocalResource> tezJarResources, LocalResource binaryConfig, - boolean tezLrsAsArchive, Map<String, String> additionalConfigs) { + boolean tezLrsAsArchive, Map<String, String> additionalConfigs, + JavaOptsChecker javaOptsChecker) { verify(true); DAGPlan.Builder dagBuilder = DAGPlan.newBuilder(); @@ -828,8 +830,15 @@ public class DAG { taskConfigBuilder.setNumTasks(vertexParallelism); taskConfigBuilder.setMemoryMb(vertexTaskResource.getMemory()); taskConfigBuilder.setVirtualCores(vertexTaskResource.getVirtualCores()); - taskConfigBuilder.setJavaOpts( - TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vertex.getTaskLaunchCmdOpts(), tezConf)); + + try { + taskConfigBuilder.setJavaOpts( + TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vertex.getTaskLaunchCmdOpts(), tezConf, + javaOptsChecker)); + } catch (TezException e) { + throw new TezUncheckedException("Invalid TaskLaunchCmdOpts defined for Vertex " + + vertex.getName() + " : " + e.getMessage(), e); + } taskConfigBuilder.setTaskModule(vertex.getName()); if (!vertexLRs.isEmpty()) { http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java ---------------------------------------------------------------------- diff --git a/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java b/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java index aa781bc..cd9e59c 100644 --- a/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java +++ b/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java @@ -1278,4 +1278,26 @@ public class TezConfiguration extends Configuration { public static final String TEZ_CLIENT_ASYNCHRONOUS_STOP = TEZ_PREFIX + "client.asynchronous-stop"; public static final boolean TEZ_CLIENT_ASYNCHRONOUS_STOP_DEFAULT = true; + + /** + * String value. + * Ability to provide a different implementation to check/verify java opts defined + * for vertices/tasks. + * Class has to be an instance of JavaOptsChecker + */ + @Private + @ConfigurationScope(Scope.CLIENT) + public static final String TEZ_CLIENT_JAVA_OPTS_CHECKER_CLASS = + TEZ_PREFIX + "java.opts.checker.class"; + + /** + * Boolean value. Default true. + * Ability to disable the Java Opts Checker + */ + @Private + @ConfigurationScope(Scope.CLIENT) + public static final String TEZ_CLIENT_JAVA_OPTS_CHECKER_ENABLED = + TEZ_PREFIX + "java.opts.checker.enabled"; + public static final boolean TEZ_CLIENT_JAVA_OPTS_CHECKER_ENABLED_DEFAULT = true; + } http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/test/java/org/apache/tez/client/TestTezClient.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/client/TestTezClient.java b/tez-api/src/test/java/org/apache/tez/client/TestTezClient.java index f1f23e1..7f44380 100644 --- a/tez-api/src/test/java/org/apache/tez/client/TestTezClient.java +++ b/tez-api/src/test/java/org/apache/tez/client/TestTezClient.java @@ -466,4 +466,27 @@ public class TestTezClient { verify(client.mockYarnClient, atLeast(2)).getApplicationReport(client.mockAppId); Assert.assertTrue("Stop ended before timeout", end - start > HARD_KILL_TIMEOUT); } + + public static class InvalidChecker { + // No-op class + } + + @Test(timeout = 5000) + public void testInvalidJavaOptsChecker1() throws YarnException, IOException, ServiceException, + TezException { + TezConfiguration conf = new TezConfiguration(); + conf.set(TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_CLASS, "InvalidClassName"); + TezClientForTest client = configureAndCreateTezClient(conf); + client.start(); + } + + @Test(timeout = 5000) + public void testInvalidJavaOptsChecker2() throws YarnException, IOException, ServiceException, + TezException { + TezConfiguration conf = new TezConfiguration(); + conf.set(TezConfiguration.TEZ_CLIENT_JAVA_OPTS_CHECKER_CLASS, InvalidChecker.class.getName()); + TezClientForTest client = configureAndCreateTezClient(conf); + client.start(); + } + } http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java b/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java index cfba917..00eb876 100644 --- a/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java +++ b/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java @@ -65,6 +65,7 @@ import org.apache.tez.dag.api.DAG; import org.apache.tez.dag.api.ProcessorDescriptor; import org.apache.tez.dag.api.TezConfiguration; import org.apache.tez.dag.api.TezConstants; +import org.apache.tez.dag.api.TezException; import org.apache.tez.dag.api.TezUncheckedException; import org.apache.tez.dag.api.Vertex; import org.apache.tez.dag.api.records.DAGProtos.ConfigurationProto; @@ -225,7 +226,7 @@ public class TestTezClientUtils { ApplicationSubmissionContext appSubmissionContext = TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf, new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(), - mock(HistoryACLPolicyManager.class)); + mock(HistoryACLPolicyManager.class), null); ContainerLaunchContext amClc = appSubmissionContext.getAMContainerSpec(); Map<String, ByteBuffer> amServiceData = amClc.getServiceData(); @@ -258,7 +259,7 @@ public class TestTezClientUtils { ApplicationSubmissionContext appSubmissionContext = TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf, new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(), - mock(HistoryACLPolicyManager.class)); + mock(HistoryACLPolicyManager.class), null); List<String> expectedCommands = new LinkedList<String>(); expectedCommands.add("-Dlog4j.configuratorClass=org.apache.tez.common.TezLog4jConfigurator"); @@ -298,7 +299,7 @@ public class TestTezClientUtils { ApplicationSubmissionContext appSubmissionContext = TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf, new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(), - mock(HistoryACLPolicyManager.class)); + mock(HistoryACLPolicyManager.class), null); List<String> expectedCommands = new LinkedList<String>(); expectedCommands.add("-Dlog4j.configuratorClass=org.apache.tez.common.TezLog4jConfigurator"); @@ -364,7 +365,7 @@ public class TestTezClientUtils { } @Test(timeout = 5000) - public void testTaskCommandOpts() { + public void testTaskCommandOpts() throws TezException { TezConfiguration tezConf = new TezConfiguration(); String taskCommandOpts = "-Xmx 200m -Dtest.property"; tezConf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, taskCommandOpts); @@ -641,4 +642,57 @@ public class TestTezClientUtils { Assert.assertTrue(resourceNames.contains("dir2-f.txt")); } + @Test(timeout = 5000) + public void testTaskLaunchCmdOptsSetup() throws TezException { + Configuration conf = new Configuration(false); + String vOpts = ""; + String opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, + TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS_DEFAULT + " " + + TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT + " " + vOpts); + + vOpts = "foo"; + opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, + TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS_DEFAULT + " " + vOpts); + + String taskOpts = "taskOpts"; + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, taskOpts); + opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, + TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS_DEFAULT + + " " + taskOpts + " " + vOpts); + + } + + @Test(timeout = 5000) + public void testClusterTaskLaunchCmdOptsSetup() throws TezException { + Configuration conf = new Configuration(false); + String adminOpts = "adminOpts"; + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CLUSTER_DEFAULT_CMD_OPTS, adminOpts); + + String vOpts = ""; + String opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, + adminOpts + " " + + TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT + " " + vOpts); + + vOpts = "foo"; + opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, adminOpts + " " + vOpts); + + String taskOpts = "taskOpts"; + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, taskOpts); + opts = TezClientUtils.addDefaultsToTaskLaunchCmdOpts(vOpts, conf); + + Assert.assertEquals(opts, adminOpts + " " + taskOpts + " " + vOpts); + + } + + } http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/test/java/org/apache/tez/common/TestJavaOptsChecker.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/common/TestJavaOptsChecker.java b/tez-api/src/test/java/org/apache/tez/common/TestJavaOptsChecker.java new file mode 100644 index 0000000..07eb9b6 --- /dev/null +++ b/tez-api/src/test/java/org/apache/tez/common/TestJavaOptsChecker.java @@ -0,0 +1,111 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tez.common; + +import org.apache.tez.dag.api.TezConfiguration; +import org.apache.tez.dag.api.TezException; +import org.apache.tez.dag.api.TezUncheckedException; +import org.junit.Assert; +import org.junit.Test; + +public class TestJavaOptsChecker { + + private final JavaOptsChecker javaOptsChecker = new JavaOptsChecker(); + + @Test(timeout = 5000) + public void testBasicChecker() throws TezException { + javaOptsChecker.checkOpts(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT); + } + + @Test(timeout = 5000) + public void testMultipleGC() { + // Clashing GC values + String opts = "-XX:+UseConcMarkSweepGC -XX:+UseG1GC -XX:+UseParallelGC "; + try { + javaOptsChecker.checkOpts(opts); + Assert.fail("Expected check to fail with opts=" + opts); + } catch (TezException e) { + Assert.assertTrue(e.getMessage(), + e.getMessage().contains("Invalid/conflicting GC options found")); + } + } + + @Test(timeout = 5000) + public void testPositiveNegativeOpts() throws TezException { + // Multiple positive GC values + String opts = "-XX:+UseConcMarkSweepGC -XX:+UseG1GC -XX:+UseParallelGC -XX:-UseG1GC "; + try { + javaOptsChecker.checkOpts(opts); + Assert.fail("Expected check to fail with opts=" + opts); + } catch (TezException e) { + Assert.assertTrue(e.getMessage(), + e.getMessage().contains("Invalid/conflicting GC options found")); + } + + // Positive following a negative is still a positive + opts = " -XX:-UseG1GC -XX:+UseParallelGC -XX:-UseG1GC -XX:+UseG1GC"; + try { + javaOptsChecker.checkOpts(opts); + Assert.fail("Expected check to fail with opts=" + opts); + } catch (TezException e) { + Assert.assertTrue(e.getMessage(), + e.getMessage().contains("Invalid/conflicting GC options found")); + } + + // Order of positive and negative matters + opts = " -XX:+UseG1GC -XX:-UseG1GC -XX:+UseParallelGC -XX:-UseG1GC -XX:+UseG1GC"; + try { + javaOptsChecker.checkOpts(opts); + Assert.fail("Expected check to fail with opts=" + opts); + } catch (TezException e) { + Assert.assertTrue(e.getMessage(), + e.getMessage().contains("Invalid/conflicting GC options found")); + } + + // Sanity check for good condition + opts = " -XX:+UseG1GC -XX:+UseParallelGC -XX:-UseG1GC "; + javaOptsChecker.checkOpts(opts); + + // Invalid negative can be ignored + opts = " -XX:+UseG1GC -XX:+UseParallelGC -XX:-UseG1GC -XX:-UseConcMarkSweepGC "; + javaOptsChecker.checkOpts(opts); + + } + + @Test(timeout = 5000) + public void testSpecialCaseNonConflictingGCOptions() throws TezException { + String opts = " -XX:+UseParNewGC -XX:+UseConcMarkSweepGC "; + javaOptsChecker.checkOpts(opts); + + opts += " -XX:-UseG1GC "; + javaOptsChecker.checkOpts(opts); + + opts += " -XX:+UseG1GC "; + try { + javaOptsChecker.checkOpts(opts); + Assert.fail("Expected check to fail with opts=" + opts); + } catch (TezException e) { + Assert.assertTrue(e.getMessage(), + e.getMessage().contains("Invalid/conflicting GC options found")); + } + + + } + +} http://git-wip-us.apache.org/repos/asf/tez/blob/27b0f2fa/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGPlan.java ---------------------------------------------------------------------- diff --git a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGPlan.java b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGPlan.java index fccbb08..e6e0232 100644 --- a/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGPlan.java +++ b/tez-api/src/test/java/org/apache/tez/dag/api/TestDAGPlan.java @@ -34,6 +34,7 @@ import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; import org.apache.hadoop.yarn.api.records.LocalResource; import org.apache.hadoop.yarn.api.records.Resource; +import org.apache.tez.common.JavaOptsChecker; import org.apache.tez.dag.api.EdgeProperty.DataMovementType; import org.apache.tez.dag.api.EdgeProperty.DataSourceType; import org.apache.tez.dag.api.EdgeProperty.SchedulingType; @@ -311,4 +312,36 @@ public class TestDAGPlan { assertNotNull(fetchedCredentials.getToken(new Text("Token1"))); assertNotNull(fetchedCredentials.getToken(new Text("Token2"))); } + + @Test(timeout = 5000) + public void testInvalidJavaOpts() { + DAG dag = DAG.create("testDag"); + ProcessorDescriptor pd1 = ProcessorDescriptor.create("processor1") + .setUserPayload(UserPayload.create(ByteBuffer.wrap("processor1Bytes".getBytes()))); + Vertex v1 = Vertex.create("v1", pd1, 10, Resource.newInstance(1024, 1)); + v1.setTaskLaunchCmdOpts(" -XX:+UseG1GC "); + + dag.addVertex(v1); + + TezConfiguration conf = new TezConfiguration(false); + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, " -XX:+UseParallelGC "); + try { + DAGPlan dagProto = dag.createDag(conf, null, null, null, true, null, + new JavaOptsChecker()); + fail("Expected dag creation to fail for invalid java opts"); + } catch (TezUncheckedException e) { + Assert.assertTrue(e.getMessage().contains("Invalid/conflicting GC options")); + } + + // Should not fail as java opts valid + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, " -XX:-UseParallelGC "); + DAGPlan dagProto1 = dag.createDag(conf, null, null, null, true, null, + new JavaOptsChecker()); + + // Should not fail as no checker enabled + conf.set(TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS, " -XX:+UseParallelGC "); + DAGPlan dagProto2 = dag.createDag(conf, null, null, null, true, null, null); + + } + }
