You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tez.apache.org by hi...@apache.org on 2015/08/28 04:15:54 UTC

tez git commit: TEZ-2662. Provide a way to check whether AM or task opts are valid and error if not. (hitesh)

Repository: tez
Updated Branches:
  refs/heads/master 6098f1bb9 -> fd714c296


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/fd714c29
Tree: http://git-wip-us.apache.org/repos/asf/tez/tree/fd714c29
Diff: http://git-wip-us.apache.org/repos/asf/tez/diff/fd714c29

Branch: refs/heads/master
Commit: fd714c296c1e33ffcdb6763ab1b67b1312f52e7a
Parents: 6098f1b
Author: Hitesh Shah <hi...@apache.org>
Authored: Thu Aug 27 19:15:34 2015 -0700
Committer: Hitesh Shah <hi...@apache.org>
Committed: Thu Aug 27 19:15:34 2015 -0700

----------------------------------------------------------------------
 CHANGES.txt                                     |   2 +
 .../java/org/apache/tez/client/TezClient.java   |  31 +++++-
 .../org/apache/tez/client/TezClientUtils.java   |  41 +++++--
 .../org/apache/tez/common/JavaOptsChecker.java  |  87 +++++++++++++++
 .../main/java/org/apache/tez/dag/api/DAG.java   |  16 ++-
 .../apache/tez/dag/api/TezConfiguration.java    |  22 ++++
 .../org/apache/tez/client/TestTezClient.java    |  22 ++++
 .../apache/tez/client/TestTezClientUtils.java   |  64 ++++++++++-
 .../apache/tez/common/TestJavaOptsChecker.java  | 111 +++++++++++++++++++
 .../org/apache/tez/dag/api/TestDAGPlan.java     |  57 ++++++++--
 10 files changed, 423 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tez/blob/fd714c29/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 0c15c1f..82fe016 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -8,6 +8,7 @@ INCOMPATIBLE CHANGES
 
 ALL CHANGES:
   TEZ-2747. Update master to reflect 0.8.0-alpha release.
+  TEZ-2662. Provide a way to check whether AM or task opts are valid and error if not.
 
 Release 0.8.0-alpha: 2015-08-29 
 
@@ -150,6 +151,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-2734. Add a test to verify the filename generated by OnDiskMerge.
   TEZ-2732. DefaultSorter throws ArrayIndex exceptions on 2047 Mb size sort buffers
   TEZ-2687. ATS History shutdown happens before the min-held containers are released

http://git-wip-us.apache.org/repos/asf/tez/blob/fd714c29/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 e3e9e74..312ddcd 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.apache.tez.common.RPCUtil;
 import org.apache.tez.common.counters.Limits;
 import org.apache.tez.serviceplugins.api.ServicePluginsDescriptor;
@@ -122,6 +123,7 @@ public class TezClient {
   @VisibleForTesting
   final ServicePluginsDescriptor servicePluginsDescriptor;
   private HistoryACLPolicyManager historyACLPolicyManager;
+  private JavaOptsChecker javaOptsChecker = null;
 
   private int preWarmDAGCounter = 0;
 
@@ -365,6 +367,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,
@@ -390,7 +414,7 @@ public class TezClient {
                 sessionAppId,
                 null, clientName, amConfig,
                 tezJarResources, sessionCredentials, usingTezArchiveDeploy, apiVersionInfo,
-                historyACLPolicyManager, servicePluginsDescriptor);
+                historyACLPolicyManager, servicePluginsDescriptor, javaOptsChecker);
   
         // Set Tez Sessions to not retry on AM crashes if recovery is disabled
         if (!amConfig.getTezConfiguration().getBoolean(
@@ -473,7 +497,8 @@ public class TezClient {
 
     Map<String, LocalResource> tezJarResources = getTezJarResources(sessionCredentials);
     DAGPlan dagPlan = TezClientUtils.prepareAndCreateDAGPlan(dag, amConfig, tezJarResources,
-        usingTezArchiveDeploy, sessionCredentials, aclConfigs, servicePluginsDescriptor);
+        usingTezArchiveDeploy, sessionCredentials, aclConfigs, servicePluginsDescriptor,
+        javaOptsChecker);
 
     SubmitDAGRequestProto.Builder requestBuilder = SubmitDAGRequestProto.newBuilder();
     requestBuilder.setDAGPlan(dagPlan).build();
@@ -807,7 +832,7 @@ public class TezClient {
           .createApplicationSubmissionContext( 
               appId, dag, dag.getName(), amConfig, tezJarResources, credentials,
               usingTezArchiveDeploy, apiVersionInfo, historyACLPolicyManager,
-              servicePluginsDescriptor);
+              servicePluginsDescriptor, javaOptsChecker);
       LOG.info("Submitting DAG to YARN"
           + ", applicationId=" + appId
           + ", dagName=" + dag.getName());

http://git-wip-us.apache.org/repos/asf/tez/blob/fd714c29/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 ecf5c07..8f1eb7f 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.apache.tez.dag.api.records.DAGProtos.AMPluginDescriptorProto;
 import org.apache.tez.serviceplugins.api.ServicePluginsDescriptor;
 import org.slf4j.Logger;
@@ -419,7 +420,7 @@ public class TezClientUtils {
       AMConfiguration amConfig, Map<String, LocalResource> tezJarResources,
       Credentials sessionCreds, boolean tezLrsAsArchive,
       TezApiVersionInfo apiVersionInfo, HistoryACLPolicyManager historyACLPolicyManager,
-      ServicePluginsDescriptor servicePluginsDescriptor)
+      ServicePluginsDescriptor servicePluginsDescriptor, JavaOptsChecker javaOptsChecker)
       throws IOException, YarnException {
 
     Preconditions.checkNotNull(sessionCreds);
@@ -609,7 +610,7 @@ public class TezClientUtils {
     if(dag != null) {
       
       DAGPlan dagPB = prepareAndCreateDAGPlan(dag, amConfig, tezJarResources, tezLrsAsArchive,
-          sessionCreds, servicePluginsDescriptor);
+          sessionCreds, servicePluginsDescriptor, javaOptsChecker);
 
       // emit protobuf DAG file style
       Path binaryPath = TezCommonUtils.getTezBinPlanStagingPath(tezSysStagingPath);
@@ -685,19 +686,22 @@ public class TezClientUtils {
   
   static DAGPlan prepareAndCreateDAGPlan(DAG dag, AMConfiguration amConfig,
       Map<String, LocalResource> tezJarResources, boolean tezLrsAsArchive,
-      Credentials credentials, ServicePluginsDescriptor servicePluginsDescriptor) throws IOException {
+      Credentials credentials, ServicePluginsDescriptor servicePluginsDescriptor,
+      JavaOptsChecker javaOptsChecker) throws IOException {
     return prepareAndCreateDAGPlan(dag, amConfig, tezJarResources, tezLrsAsArchive, credentials,
-        null, servicePluginsDescriptor);
+        null, servicePluginsDescriptor, javaOptsChecker);
   }
 
   static DAGPlan prepareAndCreateDAGPlan(DAG dag, AMConfiguration amConfig,
       Map<String, LocalResource> tezJarResources, boolean tezLrsAsArchive,
       Credentials credentials, Map<String, String> additionalDAGConfigs,
-      ServicePluginsDescriptor servicePluginsDescriptor) throws IOException {
+      ServicePluginsDescriptor servicePluginsDescriptor,
+      JavaOptsChecker javaOptsChecker) throws IOException {
     Credentials dagCredentials = setupDAGCredentials(dag, credentials,
         amConfig.getTezConfiguration());
     return dag.createDag(amConfig.getTezConfiguration(), dagCredentials, tezJarResources,
-        amConfig.getBinaryConfLR(), tezLrsAsArchive, additionalDAGConfigs, servicePluginsDescriptor);
+        amConfig.getBinaryConfLR(), tezLrsAsArchive, additionalDAGConfigs, servicePluginsDescriptor,
+        javaOptsChecker);
   }
   
   static void maybeAddDefaultLoggingJavaOpts(String logLevel, List<String> vargs) {
@@ -726,21 +730,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;
   }
 
@@ -986,6 +1008,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/fd714c29/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/fd714c29/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 78bb660..ad656cd 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
@@ -33,6 +33,7 @@ import java.util.Stack;
 import org.apache.commons.collections4.BidiMap;
 import org.apache.commons.collections4.bidimap.DualLinkedHashBidiMap;
 import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.tez.common.JavaOptsChecker;
 import org.apache.tez.dag.api.Vertex.VertexExecutionContext;
 import org.apache.tez.dag.api.records.DAGProtos;
 import org.apache.tez.serviceplugins.api.ServicePluginsDescriptor;
@@ -716,7 +717,7 @@ public class DAG {
                            Map<String, LocalResource> tezJarResources, LocalResource binaryConfig,
                            boolean tezLrsAsArchive) {
     return createDag(tezConf, extraCredentials, tezJarResources, binaryConfig, tezLrsAsArchive,
-        null, null);
+        null, null, null);
   }
 
   // create protobuf message describing DAG
@@ -724,7 +725,7 @@ public class DAG {
   public synchronized DAGPlan createDag(Configuration tezConf, Credentials extraCredentials,
       Map<String, LocalResource> tezJarResources, LocalResource binaryConfig,
       boolean tezLrsAsArchive, Map<String, String> additionalConfigs,
-                                        ServicePluginsDescriptor servicePluginsDescriptor) {
+      ServicePluginsDescriptor servicePluginsDescriptor, JavaOptsChecker javaOptsChecker) {
     verify(true);
 
     DAGPlan.Builder dagBuilder = DAGPlan.newBuilder();
@@ -873,8 +874,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/fd714c29/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 3b7378a..bb404ee 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
@@ -1264,4 +1264,26 @@ public class TezConfiguration extends Configuration {
       TEZ_PREFIX + "client.diagnostics.wait.timeout-ms";
   @Private
   public static final long TEZ_CLIENT_DIAGNOSTICS_WAIT_TIMEOUT_MS_DEFAULT = 3*1000;
+
+  /**
+   * 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/fd714c29/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 66b273a..2c3cb36 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
@@ -502,4 +502,26 @@ public class TestTezClient {
         amConf.getTezConfiguration().getBoolean(TezConfiguration.TEZ_AM_SESSION_MODE, false));
   }
 
+  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/fd714c29/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 d1033b2..394e4dd 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
@@ -66,6 +66,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 {
         appId, null, "dagname",
         amConf, m,
         credentials, false,
-        new TezApiVersionInfo(), null, null);
+        new TezApiVersionInfo(), null, null, null);
     assertEquals(testpriority, appcontext.getPriority().getPriority());
   }
 
@@ -262,7 +263,7 @@ public class TestTezClientUtils {
     ApplicationSubmissionContext appSubmissionContext =
         TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf,
             new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(),
-            mock(HistoryACLPolicyManager.class), null);
+            mock(HistoryACLPolicyManager.class), null, null);
 
     ContainerLaunchContext amClc = appSubmissionContext.getAMContainerSpec();
     Map<String, ByteBuffer> amServiceData = amClc.getServiceData();
@@ -295,7 +296,7 @@ public class TestTezClientUtils {
     ApplicationSubmissionContext appSubmissionContext =
         TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf,
             new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(),
-            mock(HistoryACLPolicyManager.class), null);
+            mock(HistoryACLPolicyManager.class), null, null);
 
     List<String> expectedCommands = new LinkedList<String>();
     expectedCommands.add("-Dlog4j.configuratorClass=org.apache.tez.common.TezLog4jConfigurator");
@@ -335,7 +336,7 @@ public class TestTezClientUtils {
     ApplicationSubmissionContext appSubmissionContext =
         TezClientUtils.createApplicationSubmissionContext(appId, dag, "amName", amConf,
             new HashMap<String, LocalResource>(), credentials, false, new TezApiVersionInfo(),
-            mock(HistoryACLPolicyManager.class), null);
+            mock(HistoryACLPolicyManager.class), null, null);
 
     List<String> expectedCommands = new LinkedList<String>();
     expectedCommands.add("-Dlog4j.configuratorClass=org.apache.tez.common.TezLog4jConfigurator");
@@ -401,7 +402,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);
@@ -691,4 +692,57 @@ public class TestTezClientUtils {
     assertTrue(confProto.getAmPluginDescriptor().getUberEnabled());
   }
 
+  @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/fd714c29/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/fd714c29/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 7edea2f..005c027 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;
@@ -327,7 +328,7 @@ public class TestDAGPlan {
     dag.addVertex(v1);
 
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, null);
+      dag.createDag(new TezConfiguration(false), null, null, null, true);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("AM execution"));
@@ -336,7 +337,7 @@ public class TestDAGPlan {
     dag.setExecutionContext(VertexExecutionContext.createExecuteInContainers(true));
 
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, null);
+      dag.createDag(new TezConfiguration(false), null, null, null, true);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("container execution"));
@@ -370,13 +371,14 @@ public class TestDAGPlan {
 
     // Should succeed. Default context is containers.
     dag.createDag(new TezConfiguration(false), null, null, null, true, null,
-        servicePluginsDescriptor);
+        servicePluginsDescriptor, null);
 
 
     // Set execute in AM should fail
     v1.setExecutionContext(VertexExecutionContext.createExecuteInAm(true));
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, servicePluginsDescriptor);
+      dag.createDag(new TezConfiguration(false), null, null, null, true, null,
+          servicePluginsDescriptor, null);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("AM execution"));
@@ -384,12 +386,14 @@ public class TestDAGPlan {
 
     // Valid context
     v1.setExecutionContext(validExecContext);
-    dag.createDag(new TezConfiguration(false), null, null, null, true, null, servicePluginsDescriptor);
+    dag.createDag(new TezConfiguration(false), null, null, null, true, null,
+        servicePluginsDescriptor, null);
 
     // Invalid task scheduler
     v1.setExecutionContext(invalidExecContext1);
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, servicePluginsDescriptor);
+      dag.createDag(new TezConfiguration(false), null, null, null, true, null,
+          servicePluginsDescriptor, null);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("testvertex"));
@@ -400,7 +404,8 @@ public class TestDAGPlan {
     // Invalid ContainerLauncher
     v1.setExecutionContext(invalidExecContext2);
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, servicePluginsDescriptor);
+      dag.createDag(new TezConfiguration(false), null, null, null, true, null,
+          servicePluginsDescriptor, null);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("testvertex"));
@@ -411,7 +416,8 @@ public class TestDAGPlan {
     // Invalid task comm
     v1.setExecutionContext(invalidExecContext3);
     try {
-      dag.createDag(new TezConfiguration(false), null, null, null, true, null, servicePluginsDescriptor);
+      dag.createDag(new TezConfiguration(false), null, null, null, true, null,
+          servicePluginsDescriptor, null);
       fail("Expecting dag create to fail due to invalid ServicePluginDescriptor");
     } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("testvertex"));
@@ -456,7 +462,8 @@ public class TestDAGPlan {
     dag.addVertex(v1).addVertex(v2).addEdge(edge);
     dag.setExecutionContext(defaultExecutionContext);
 
-    DAGPlan dagProto = dag.createDag(new TezConfiguration(), null, null, null, true, null, servicePluginsDescriptor);
+    DAGPlan dagProto = dag.createDag(new TezConfiguration(), null, null, null, true, null,
+        servicePluginsDescriptor, null);
 
     assertEquals(2, dagProto.getVertexCount());
     assertEquals(1, dagProto.getEdgeCount());
@@ -481,4 +488,36 @@ public class TestDAGPlan {
     VertexPlan v2Proto = dagProto.getVertex(1);
     assertFalse(v2Proto.hasExecutionContext());
   }
+
+  @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, 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, 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, null);
+
+  }
+
 }