hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acmur...@apache.org
Subject svn commit: r1357474 - in /hadoop/common/branches/branch-1-win: ./ src/mapred/org/apache/hadoop/mapred/ src/test/org/apache/hadoop/mapred/
Date Thu, 05 Jul 2012 07:03:08 GMT
Author: acmurthy
Date: Thu Jul  5 07:03:07 2012
New Revision: 1357474

URL: http://svn.apache.org/viewvc?rev=1357474&view=rev
Log:
MAPREDUCE-4332 Fix command-length abort issues on Windows. Contributed by Ivan Mitic.

Added:
    hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/mapred/TestTaskLog.java
Modified:
    hadoop/common/branches/branch-1-win/CHANGES.txt
    hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java
    hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskLog.java
    hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskRunner.java

Modified: hadoop/common/branches/branch-1-win/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.txt?rev=1357474&r1=1357473&r2=1357474&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/CHANGES.txt (original)
+++ hadoop/common/branches/branch-1-win/CHANGES.txt Thu Jul  5 07:03:07 2012
@@ -50,6 +50,9 @@ branch-hadoop-1-win - unreleased
 
     HADOOP-8487 Many HDFS tests use a test path intended for local file system tests (Ivan
Mitic via Sanjay Radia)
 
+    MAPREDUCE-4332 Fix command-length abort issues on Windows. (Ivan Mitic via
+    acmurthy)
+
 Release 1.1.0 - unreleased
 
   NEW FEATURES

Modified: hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java?rev=1357474&r1=1357473&r2=1357474&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java
(original)
+++ hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java
Thu Jul  5 07:03:07 2012
@@ -142,6 +142,7 @@ public class DefaultTaskController exten
       shExec.execute();
     } catch (Exception e) {
       if (shExec == null) {
+        LOG.warn("Failed to launch the task with: " + e);
         return -1;
       }
       int exitCode = shExec.getExitCode();

Modified: hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskLog.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskLog.java?rev=1357474&r1=1357473&r2=1357474&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskLog.java (original)
+++ hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskLog.java Thu
Jul  5 07:03:07 2012
@@ -65,7 +65,16 @@ public class TaskLog {
     LogFactory.getLog(TaskLog.class);
 
   static final String USERLOGS_DIR_NAME = "userlogs";
-
+  
+  // Windows has relatively small hard limit on the command line length.
+  // On non-Windows platforms, we don't enforce any limits as command
+  // line usually supports over 200k (or even million) characters
+  // and it is very unlikely that we can hit this limit. Another option is
+  // to use "getconf ARG_MAX" to extract this value, however, since this has
+  // perf impact that we'd pay for scenario that is likely not
+  // to happen, we are staying out of it (until proved otherwise).
+  static int MAX_CMD_LINE_LENGTH = Shell.WINDOWS ? 8192 : Integer.MAX_VALUE;
+  
   private static final File LOG_DIR = 
     new File(getBaseLogDir(), USERLOGS_DIR_NAME).getAbsoluteFile();
   
@@ -481,7 +490,10 @@ public class TaskLog {
     }
   }
 
-  private static final String bashCommand = (Shell.WINDOWS)? "cmd": "bash";
+  private static final String shellCommand = (Shell.WINDOWS)? "cmd": "bash";
+  private static final String shellCommandSufix = (Shell.WINDOWS) ? "/c" : "-c";
+  private static final String shellCommandNullOutput =
+      (Shell.WINDOWS) ? "< nul" : "< /dev/null";
   private static final String tailCommand = "tail";
   
   /**
@@ -609,8 +621,8 @@ public class TaskLog {
       boolean useSetsid
      ) throws IOException {
     List<String> result = new ArrayList<String>(3);
-    result.add(bashCommand);
-    result.add((Shell.WINDOWS)? "/c" : "-c");
+    result.add(shellCommand);
+    result.add(shellCommandSufix);
     String mergedCmd = buildCommandLine(setup,
         cmd,
         stdoutFilename,
@@ -631,48 +643,71 @@ public class TaskLog {
     String stdout = FileUtil.makeShellPath(stdoutFilename);
     String stderr = FileUtil.makeShellPath(stderrFilename);
     StringBuilder mergedCmd = new StringBuilder();
-    
+
     if (!Shell.WINDOWS) {
       mergedCmd.append("export JVM_PID=`echo $$`\n");
     }
 
     if (setup != null) {
       for (String s : setup) {
+        if (s.length() > MAX_CMD_LINE_LENGTH) {
+          throw new IOException("Command exceeds the OS command length limit: "
+                                + MAX_CMD_LINE_LENGTH + ", command: \""
+                                + s + "\"");
+        }
+
         mergedCmd.append(s);
         mergedCmd.append("\n");
       }
     }
+    StringBuilder runCommand = new StringBuilder();
+
     if (tailLength > 0) {
-      mergedCmd.append("(");
+      runCommand.append("(");
     } else if (ProcessTree.isSetsidAvailable && useSetSid 
         && !Shell.WINDOWS) {
-      mergedCmd.append("exec setsid ");
+      runCommand.append("exec setsid ");
     } else {
       if (!Shell.WINDOWS)
-        mergedCmd.append("exec ");
+        runCommand.append("exec ");
     }
-    mergedCmd.append(addCommand(cmd, true));
-    mergedCmd.append((Shell.WINDOWS)? " < nul ":" < /dev/null ");
+    runCommand.append(addCommand(cmd, true));
+    runCommand.append(" ");
+    runCommand.append(shellCommandNullOutput);
+    runCommand.append(" ");
     if (tailLength > 0) {
-      mergedCmd.append(" | ");
-      mergedCmd.append(tailCommand);
-      mergedCmd.append((Shell.WINDOWS)?" /c ":" -c ");
-      mergedCmd.append(tailLength);
-      mergedCmd.append(" >> ");
-      mergedCmd.append(stdout);
-      mergedCmd.append(" ; exit $PIPESTATUS ) 2>&1 | ");
-      mergedCmd.append(tailCommand);
-      mergedCmd.append((Shell.WINDOWS)? " /c ":" -c ");
-      mergedCmd.append(tailLength);
-      mergedCmd.append(" >> ");
-      mergedCmd.append(stderr);
-      mergedCmd.append(" ; exit $PIPESTATUS");
+      runCommand.append(" | ");
+      runCommand.append(tailCommand);
+      runCommand.append(" ");
+      runCommand.append(shellCommandSufix);
+      runCommand.append(" ");
+      runCommand.append(tailLength);
+      runCommand.append(" >> ");
+      runCommand.append(stdout);
+      runCommand.append(" ; exit $PIPESTATUS ) 2>&1 | ");
+      runCommand.append(tailCommand);
+      runCommand.append(" ");
+      runCommand.append(shellCommandSufix);
+      runCommand.append(" ");
+      runCommand.append(tailLength);
+      runCommand.append(" >> ");
+      runCommand.append(stderr);
+      runCommand.append(" ; exit $PIPESTATUS");
     } else {
-      mergedCmd.append(" 1>> ");
-      mergedCmd.append(stdout);
-      mergedCmd.append(" 2>> ");
-      mergedCmd.append(stderr);
+      runCommand.append(" 1>> ");
+      runCommand.append(stdout);
+      runCommand.append(" 2>> ");
+      runCommand.append(stderr);
+    }
+
+    if (runCommand.length() > MAX_CMD_LINE_LENGTH) {
+      throw new IOException("Command exceeds the OS command length limit: "
+                            + MAX_CMD_LINE_LENGTH + ", command: \""
+                            + runCommand + "\"");
     }
+
+    mergedCmd.append(runCommand);
+
     return mergedCmd.toString();
   }
 
@@ -730,8 +765,8 @@ public class TaskLog {
                                             ) throws IOException {
     String debugout = FileUtil.makeShellPath(debugoutFilename);
     List<String> result = new ArrayList<String>(3);
-    result.add(bashCommand);
-    result.add((Shell.WINDOWS)? "/c" : "-c");
+    result.add(shellCommand);
+    result.add(shellCommandSufix);
     StringBuffer mergedCmd = new StringBuffer();
     mergedCmd.append("exec ");
     boolean isExecutable = true;

Modified: hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskRunner.java?rev=1357474&r1=1357473&r2=1357474&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
(original)
+++ hadoop/common/branches/branch-1-win/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
Thu Jul  5 07:03:07 2012
@@ -75,6 +75,8 @@ abstract class TaskRunner extends Thread
   
   static final String HADOOP_WORK_DIR = "HADOOP_WORK_DIR";
   
+  static final String JAVA_CLASSPATH = "CLASSPATH";
+  
   static final String MAPRED_ADMIN_USER_ENV =
     "mapreduce.admin.user.env";
 
@@ -212,13 +214,15 @@ abstract class TaskRunner extends Thread
       }
       
       // Accumulates class paths for child.
-      List<String> classPaths = getClassPaths(conf, workDir,
-                                              taskDistributedCacheManager);
+      List<String> classPathsList = getClassPaths(conf, workDir,
+                                                  taskDistributedCacheManager);
+      String classPaths = StringUtils.join(SYSTEM_PATH_SEPARATOR,
+                                           classPathsList);
 
       long logSize = TaskLog.getTaskLogLength(conf);
       
       //  Build exec child JVM args.
-      Vector<String> vargs = getVMArgs(taskid, workDir, classPaths, logSize);
+      Vector<String> vargs = getVMArgs(taskid, workDir, logSize);
       
       tracker.addToMemoryManager(t.getTaskID(), t.isMapTask(), conf);
 
@@ -232,8 +236,8 @@ abstract class TaskRunner extends Thread
                  stderr);
       
       Map<String, String> env = new HashMap<String, String>();
-      errorInfo = getVMEnvironment(errorInfo, user, workDir, conf, env, taskid,
-                                   logSize);
+      errorInfo = getVMEnvironment(errorInfo, user, workDir, classPaths,
+                                   conf, env, taskid, logSize);
       
       // flatten the env as a set of export commands
       List <String> setupCmds = new ArrayList<String>();
@@ -369,7 +373,7 @@ abstract class TaskRunner extends Thread
    * @throws IOException
    */
   private Vector<String> getVMArgs(TaskAttemptID taskid, File workDir,
-      List<String> classPaths, long logSize)
+      long logSize)
       throws IOException {
     Vector<String> vargs = new Vector<String>(8);
     File jvm =                                  // use same jvm as parent
@@ -443,11 +447,6 @@ abstract class TaskRunner extends Thread
     Path childTmpDir = createChildTmpDir(workDir, conf, false);
     vargs.add("-Djava.io.tmpdir=" + childTmpDir);
 
-    // Add classpath.
-    vargs.add("-classpath");
-    String classPath = StringUtils.join(SYSTEM_PATH_SEPARATOR, classPaths);
-    vargs.add(classPath);
-
     // Setup the log4j prop
     setupLog4jProperties(vargs, taskid, logSize);
 
@@ -551,9 +550,10 @@ abstract class TaskRunner extends Thread
     return classPaths;
   }
 
-  private String getVMEnvironment(String errorInfo, String user, File workDir, 
-                                  JobConf conf, Map<String, String> env, 
-                                  TaskAttemptID taskid, long logSize
+  private String getVMEnvironment(String errorInfo, String user, File workDir,
+                                  String classPaths, JobConf conf,
+                                  Map<String, String> env, TaskAttemptID taskid,
+                                  long logSize
                                   ) throws Throwable {
     StringBuffer ldLibraryPath = new StringBuffer();
     ldLibraryPath.append(workDir.toString());
@@ -565,6 +565,7 @@ abstract class TaskRunner extends Thread
     }
     env.put("LD_LIBRARY_PATH", ldLibraryPath.toString());
     env.put(HADOOP_WORK_DIR, workDir.toString());
+    env.put(JAVA_CLASSPATH, classPaths.toString());
     //update user configured login-shell properties
     updateUserLoginEnv(errorInfo, user, conf, env);
     // put jobTokenFile name into env

Added: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/mapred/TestTaskLog.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/mapred/TestTaskLog.java?rev=1357474&view=auto
==============================================================================
--- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/mapred/TestTaskLog.java
(added)
+++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/mapred/TestTaskLog.java
Thu Jul  5 07:03:07 2012
@@ -0,0 +1,126 @@
+package org.apache.hadoop.mapred;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.hadoop.util.ProcessTree;
+import org.apache.hadoop.util.Shell;
+
+import junit.framework.TestCase;
+
+/**
+ * Tests for the TaskLog.
+ */
+public class TestTaskLog extends TestCase {
+  private List<String> cmd;
+  private List<String> setup;
+  private File stdoutFilename = new File("stdout");
+  private File stderrFilename = new File("stderr");
+  
+  /**
+   * Setup
+   */
+  public void setUp() throws Exception {
+    cmd = new LinkedList<String>();
+    setup = new LinkedList<String>();
+    setup.add("set setup");
+    cmd.add("command1");
+    cmd.add("command2");
+  }
+
+  /**
+   * Verify that TaskLog.buildCommandLine() returns the expected results.
+   */
+  public void testBuildCommandLine() throws IOException {
+    // useSetSid set to false 
+    String result = TaskLog.buildCommandLine(setup, cmd, stdoutFilename,
+      stderrFilename, 0, false);
+
+    if (Shell.WINDOWS) {
+      assertEquals("set setup\n" +
+          "command1 \"command2\"  < nul  1>> stdout 2>> stderr",
+          result);
+    } else {
+      assertEquals("export JVM_PID=`echo $$`\n" +
+          "set setup\n" +
+          "exec 'command1' 'command2'  < /dev/null  1>> stdout 2>> stderr",
+          result);
+    }
+
+    // useSetSid set to true
+    boolean setsidAvailableOrig = ProcessTree.isSetsidAvailable;
+    try {
+      ProcessTree.isSetsidAvailable = true;
+      result = TaskLog.buildCommandLine(setup, cmd, stdoutFilename,
+        stderrFilename, 0, true);
+
+      if (Shell.WINDOWS) {
+        assertEquals("set setup\n" +
+            "command1 \"command2\"  < nul  1>> stdout 2>> stderr",
+            result);
+      } else {
+        assertEquals("export JVM_PID=`echo $$`\n" +
+            "set setup\n" +
+            "exec setsid 'command1' 'command2'  < /dev/null  1>> stdout 2>>
stderr",
+            result);
+      }
+    } finally {
+      // restore the original value back
+      ProcessTree.isSetsidAvailable = setsidAvailableOrig;
+    }
+  }
+
+  private String generateString(String prefix, int length) {
+    StringBuffer result = new StringBuffer();
+    result.append(prefix);
+    for (int i = 0; i < length; ++i) {
+      result.append("c");
+    }
+    return result.toString();
+  }
+
+  /**
+   * Verify that TaskLog.buildCommandLine() throws if the command line
+   * exceeds the command line limit.
+   */
+  public void testBuildCommandLineLongCommand() {
+    int maxCmdLineLengthOrig = TaskLog.MAX_CMD_LINE_LENGTH;
+    TaskLog.MAX_CMD_LINE_LENGTH = 1000;
+    try {
+      // Test long command in setup
+      String setupTestCommand = generateString("setup ",
+        TaskLog.MAX_CMD_LINE_LENGTH + 1);
+      setup.add(setupTestCommand);
+
+      IOException ex = null;
+      try {
+        TaskLog.buildCommandLine(setup, cmd, stdoutFilename, stderrFilename,
+          0, false);
+      } catch (IOException e) {
+        ex = e;
+      }
+      assertNotNull(ex);
+      assertTrue(ex.getMessage().contains(setupTestCommand));
+
+      // Test long command in cmd
+      setup.clear();
+      String cmdTestCommand = generateString("command ",
+        TaskLog.MAX_CMD_LINE_LENGTH + 1);
+      cmd.add(cmdTestCommand.toString());
+
+      ex = null;
+      try {
+        TaskLog.buildCommandLine(setup, cmd, stdoutFilename, stderrFilename,
+          0, false);
+      } catch (IOException e) {
+        ex = e;
+      }
+      assertNotNull(ex);
+      assertTrue(ex.getMessage().contains(cmdTestCommand));
+    } finally {
+      // restore the original value back
+      TaskLog.MAX_CMD_LINE_LENGTH = maxCmdLineLengthOrig;
+    }
+  }
+}



Mime
View raw message