tez-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hit...@apache.org
Subject tez git commit: TEZ-2662. Provide a way to check whether AM or task opts are valid and error if not. (hitesh)
Date Fri, 28 Aug 2015 17:56:48 GMT
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 <hitesh@apache.org>
Authored: Fri Aug 28 10:56:34 2015 -0700
Committer: Hitesh Shah <hitesh@apache.org>
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);
+
+  }
+
 }


Mime
View raw message