You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by wa...@apache.org on 2017/09/29 23:46:06 UTC

[1/2] hadoop git commit: YARN-6550. Capture launch_container.sh logs to a separate log file. (Suma Shivaprasad via wangda)

Repository: hadoop
Updated Branches:
  refs/heads/trunk 373d0a519 -> ec2ae3060


YARN-6550. Capture launch_container.sh logs to a separate log file. (Suma Shivaprasad via wangda)

Change-Id: I0ee0b1bb459437432a22cf68861a6354f0decabb


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/febeead5
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/febeead5
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/febeead5

Branch: refs/heads/trunk
Commit: febeead5f95c6fc245ea3735f5b538d4bb4dc8a4
Parents: 373d0a5
Author: Wangda Tan <wa...@apache.org>
Authored: Fri Sep 29 16:39:46 2017 -0700
Committer: Wangda Tan <wa...@apache.org>
Committed: Fri Sep 29 16:39:46 2017 -0700

----------------------------------------------------------------------
 .../server/nodemanager/ContainerExecutor.java   |  20 +-
 .../nodemanager/DefaultContainerExecutor.java   |   3 +-
 .../nodemanager/LinuxContainerExecutor.java     |   3 +-
 .../launcher/ContainerLaunch.java               | 189 ++++++++++++++++---
 .../launcher/TestContainerLaunch.java           | 115 ++++++++++-
 5 files changed, 290 insertions(+), 40 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/febeead5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
index da50d7a..5fd059d 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
@@ -64,6 +64,9 @@ import org.apache.hadoop.yarn.server.nodemanager.util.ProcessIdFileReader;
 import org.apache.hadoop.util.Shell;
 import org.apache.hadoop.util.StringUtils;
 
+import static org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR;
+import static org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT;
+
 /**
  * This class is abstraction of the mechanism used to launch a container on the
  * underlying OS.  All executor implementations must extend ContainerExecutor.
@@ -330,6 +333,14 @@ public abstract class ContainerExecutor implements Configurable {
       String user, String outFilename) throws IOException {
     ContainerLaunch.ShellScriptBuilder sb =
         ContainerLaunch.ShellScriptBuilder.create();
+
+    // Add "set -o pipefail -e" to validate launch_container script.
+    sb.setExitOnFailure();
+
+    //Redirect stdout and stderr for launch_container script
+    sb.stdout(logDir, CONTAINER_PRE_LAUNCH_STDOUT);
+    sb.stderr(logDir, CONTAINER_PRE_LAUNCH_STDERR);
+
     Set<String> whitelist = new HashSet<>();
 
     String[] nmWhiteList = conf.get(YarnConfiguration.NM_ENV_WHITELIST,
@@ -338,10 +349,8 @@ public abstract class ContainerExecutor implements Configurable {
       whitelist.add(param);
     }
 
-    // Add "set -o pipefail -e" to validate launch_container script.
-    sb.setExitOnFailure();
-
     if (environment != null) {
+      sb.echo("Setting up env variables");
       for (Map.Entry<String, String> env : environment.entrySet()) {
         if (!whitelist.contains(env.getKey())) {
           sb.env(env.getKey(), env.getValue());
@@ -352,6 +361,7 @@ public abstract class ContainerExecutor implements Configurable {
     }
 
     if (resources != null) {
+      sb.echo("Setting up job resources");
       for (Map.Entry<Path, List<String>> resourceEntry :
           resources.entrySet()) {
         for (String linkName : resourceEntry.getValue()) {
@@ -373,15 +383,15 @@ public abstract class ContainerExecutor implements Configurable {
     if (getConf() != null &&
         getConf().getBoolean(YarnConfiguration.NM_LOG_CONTAINER_DEBUG_INFO,
         YarnConfiguration.DEFAULT_NM_LOG_CONTAINER_DEBUG_INFO)) {
+      sb.echo("Copying debugging information");
       sb.copyDebugInformation(new Path(outFilename),
           new Path(logDir, outFilename));
       sb.listDebugInformation(new Path(logDir, DIRECTORY_CONTENTS));
     }
-
+    sb.echo("Launching container");
     sb.command(command);
 
     PrintStream pout = null;
-
     try {
       pout = new PrintStream(out, false, "UTF-8");
       sb.write(pout);

http://git-wip-us.apache.org/repos/asf/hadoop/blob/febeead5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
index 8c58056..2b32234 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
@@ -320,8 +320,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
           builder.append("Exception message: ");
           builder.append(e.getMessage()).append("\n");
         }
-        builder.append("Stack trace: ");
-        builder.append(StringUtils.stringifyException(e)).append("\n");
+
         if (!shExec.getOutput().isEmpty()) {
           builder.append("Shell output: ");
           builder.append(shExec.getOutput()).append("\n");

http://git-wip-us.apache.org/repos/asf/hadoop/blob/febeead5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
index 2971f83..9e004e7 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
@@ -532,8 +532,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
         if (!Optional.fromNullable(e.getErrorOutput()).or("").isEmpty()) {
           builder.append("Exception message: " + e.getErrorOutput() + "\n");
         }
-        builder.append("Stack trace: "
-            + StringUtils.stringifyException(e) + "\n");
+        //Skip stack trace
         String output = e.getOutput();
         if (output != null && !e.getOutput().isEmpty()) {
           builder.append("Shell output: " + output + "\n");

http://git-wip-us.apache.org/repos/asf/hadoop/blob/febeead5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
index e254887..f929dfc 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
@@ -87,6 +87,8 @@ import org.apache.hadoop.yarn.util.Apps;
 import org.apache.hadoop.yarn.util.AuxiliaryServiceHelper;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.apache.hadoop.yarn.util.ConverterUtils;
 
 public class ContainerLaunch implements Callable<Integer> {
@@ -94,8 +96,13 @@ public class ContainerLaunch implements Callable<Integer> {
   private static final Logger LOG =
        LoggerFactory.getLogger(ContainerLaunch.class);
 
+  private static final String CONTAINER_PRE_LAUNCH_PREFIX = "prelaunch";
+  public static final String CONTAINER_PRE_LAUNCH_STDOUT = CONTAINER_PRE_LAUNCH_PREFIX + ".out";
+  public static final String CONTAINER_PRE_LAUNCH_STDERR = CONTAINER_PRE_LAUNCH_PREFIX + ".err";
+
   public static final String CONTAINER_SCRIPT =
     Shell.appendScriptExtension("launch_container");
+
   public static final String FINAL_CONTAINER_TOKENS_FILE = "container_tokens";
 
   private static final String PID_FILE_NAME_FMT = "%s.pid";
@@ -147,7 +154,7 @@ public class ContainerLaunch implements Callable<Integer> {
       Path containerLogDir) {
     var = var.replace(ApplicationConstants.LOG_DIR_EXPANSION_VAR,
       containerLogDir.toString());
-    var =  var.replace(ApplicationConstants.CLASS_PATH_SEPARATOR,
+    var = var.replace(ApplicationConstants.CLASS_PATH_SEPARATOR,
       File.pathSeparator);
 
     // replace parameter expansion marker. e.g. {{VAR}} on Windows is replaced
@@ -371,7 +378,7 @@ public class ContainerLaunch implements Callable<Integer> {
     String relativeContainerLogDir = ContainerLaunch
         .getRelativeContainerLogDir(appIdStr, containerIdStr);
 
-    for(String logDir : logDirs) {
+    for (String logDir : logDirs) {
       containerLogDirs.add(logDir + Path.SEPARATOR + relativeContainerLogDir);
     }
 
@@ -516,6 +523,7 @@ public class ContainerLaunch implements Callable<Integer> {
    * Tries to tail and fetch TAIL_SIZE_IN_BYTES of data from the error log.
    * ErrorLog filename is not fixed and depends upon app, hence file name
    * pattern is used.
+   *
    * @param containerID
    * @param ret
    * @param containerLogDir
@@ -524,20 +532,46 @@ public class ContainerLaunch implements Callable<Integer> {
   @SuppressWarnings("unchecked")
   protected void handleContainerExitWithFailure(ContainerId containerID,
       int ret, Path containerLogDir, StringBuilder diagnosticInfo) {
-    LOG.warn(diagnosticInfo.toString());
+    LOG.warn("Container launch failed : " + diagnosticInfo.toString());
+
+    FileSystem fileSystem = null;
+    long tailSizeInBytes =
+        conf.getLong(YarnConfiguration.NM_CONTAINER_STDERR_BYTES,
+            YarnConfiguration.DEFAULT_NM_CONTAINER_STDERR_BYTES);
+
+    // Append container prelaunch stderr to diagnostics
+    try {
+      fileSystem = FileSystem.getLocal(conf).getRaw();
+      FileStatus preLaunchErrorFileStatus = fileSystem
+          .getFileStatus(new Path(containerLogDir, ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR));
+
+      Path errorFile = preLaunchErrorFileStatus.getPath();
+      long fileSize = preLaunchErrorFileStatus.getLen();
+
+      diagnosticInfo.append("Error file: ")
+          .append(ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR).append(".\n");
+      ;
 
+      byte[] tailBuffer = tailFile(errorFile, fileSize, tailSizeInBytes);
+      diagnosticInfo.append("Last ").append(tailSizeInBytes)
+          .append(" bytes of ").append(errorFile.getName()).append(" :\n")
+          .append(new String(tailBuffer, StandardCharsets.UTF_8));
+    } catch (IOException e) {
+      LOG.error("Failed to get tail of the container's prelaunch error log file", e);
+    }
+
+    // Append container stderr to diagnostics
     String errorFileNamePattern =
         conf.get(YarnConfiguration.NM_CONTAINER_STDERR_PATTERN,
             YarnConfiguration.DEFAULT_NM_CONTAINER_STDERR_PATTERN);
-    FSDataInputStream errorFileIS = null;
+
     try {
-      FileSystem fileSystem = FileSystem.getLocal(conf).getRaw();
+      if (fileSystem == null) {
+        fileSystem = FileSystem.getLocal(conf).getRaw();
+      }
       FileStatus[] errorFileStatuses = fileSystem
           .globStatus(new Path(containerLogDir, errorFileNamePattern));
       if (errorFileStatuses != null && errorFileStatuses.length != 0) {
-        long tailSizeInBytes =
-            conf.getLong(YarnConfiguration.NM_CONTAINER_STDERR_BYTES,
-                YarnConfiguration.DEFAULT_NM_CONTAINER_STDERR_BYTES);
         Path errorFile = errorFileStatuses[0].getPath();
         long fileSize = errorFileStatuses[0].getLen();
 
@@ -560,32 +594,40 @@ public class ContainerLaunch implements Callable<Integer> {
               .append(StringUtils.join(", ", errorFileNames)).append(".\n");
         }
 
-        long startPosition =
-            (fileSize < tailSizeInBytes) ? 0 : fileSize - tailSizeInBytes;
-        int bufferSize =
-            (int) ((fileSize < tailSizeInBytes) ? fileSize : tailSizeInBytes);
-        byte[] tailBuffer = new byte[bufferSize];
-        errorFileIS = fileSystem.open(errorFile);
-        errorFileIS.readFully(startPosition, tailBuffer);
-
+        byte[] tailBuffer = tailFile(errorFile, fileSize, tailSizeInBytes);
         String tailBufferMsg = new String(tailBuffer, StandardCharsets.UTF_8);
         diagnosticInfo.append("Last ").append(tailSizeInBytes)
             .append(" bytes of ").append(errorFile.getName()).append(" :\n")
             .append(tailBufferMsg).append("\n")
             .append(analysesErrorMsgOfContainerExitWithFailure(tailBufferMsg));
+
       }
     } catch (IOException e) {
       LOG.error("Failed to get tail of the container's error log file", e);
-    } finally {
-      IOUtils.cleanupWithLogger(LOG, errorFileIS);
     }
-
     this.dispatcher.getEventHandler()
         .handle(new ContainerExitEvent(containerID,
             ContainerEventType.CONTAINER_EXITED_WITH_FAILURE, ret,
             diagnosticInfo.toString()));
   }
 
+  private byte[] tailFile(Path filePath, long fileSize, long tailSizeInBytes) throws IOException {
+    FSDataInputStream errorFileIS = null;
+    FileSystem fileSystem = FileSystem.getLocal(conf).getRaw();
+    try {
+      long startPosition =
+          (fileSize < tailSizeInBytes) ? 0 : fileSize - tailSizeInBytes;
+      int bufferSize =
+          (int) ((fileSize < tailSizeInBytes) ? fileSize : tailSizeInBytes);
+      byte[] tailBuffer = new byte[bufferSize];
+      errorFileIS = fileSystem.open(filePath);
+      errorFileIS.readFully(startPosition, tailBuffer);
+      return tailBuffer;
+    } finally {
+      IOUtils.cleanupWithLogger(LOG, errorFileIS);
+    }
+  }
+
   private String analysesErrorMsgOfContainerExitWithFailure(String errorMsg) {
     StringBuilder analysis = new StringBuilder();
     if (errorMsg.indexOf("Error: Could not find or load main class"
@@ -982,8 +1024,48 @@ public class ContainerLaunch implements Callable<Integer> {
 
     public abstract void whitelistedEnv(String key, String value) throws IOException;
 
+    protected static final String ENV_PRELAUNCH_STDOUT = "PRELAUNCH_OUT";
+    protected static final String ENV_PRELAUNCH_STDERR = "PRELAUNCH_ERR";
+
+    private boolean redirectStdOut = false;
+    private boolean redirectStdErr = false;
+
+    /**
+     * Set stdout for the shell script
+     * @param stdoutDir stdout must be an absolute path
+     * @param stdOutFile stdout file name
+     * @throws IOException thrown when stdout path is not absolute
+     */
+    public final void stdout(Path stdoutDir, String stdOutFile) throws IOException {
+      if (!stdoutDir.isAbsolute()) {
+        throw new IOException("Stdout path must be absolute");
+      }
+      redirectStdOut = true;
+      setStdOut(new Path(stdoutDir, stdOutFile));
+    }
+
+    /**
+     * Set stderr for the shell script
+     * @param stderrDir stderr must be an absolute path
+     * @param stdErrFile stderr file name
+     * @throws IOException thrown when stderr path is not absolute
+     */
+    public final void stderr(Path stderrDir, String stdErrFile) throws IOException {
+      if (!stderrDir.isAbsolute()) {
+        throw new IOException("Stdout path must be absolute");
+      }
+      redirectStdErr = true;
+      setStdErr(new Path(stderrDir, stdErrFile));
+    }
+
+    protected abstract void setStdOut(Path stdout) throws IOException;
+
+    protected abstract void setStdErr(Path stdout) throws IOException;
+
     public abstract void env(String key, String value) throws IOException;
 
+    public abstract void echo(String echoStr) throws IOException;
+
     public final void symlink(Path src, Path dst) throws IOException {
       if (!src.isAbsolute()) {
         throw new IOException("Source must be absolute");
@@ -1028,13 +1110,21 @@ public class ContainerLaunch implements Callable<Integer> {
       out.append(sb);
     }
 
-    protected final void line(String... command) {
+    protected final void buildCommand(String... command) {
       for (String s : command) {
         sb.append(s);
       }
+    }
+
+    protected final void linebreak(String... command) {
       sb.append(LINE_SEPARATOR);
     }
 
+    protected final void line(String... command) {
+      buildCommand(command);
+      linebreak();
+    }
+
     public void setExitOnFailure() {
       // Dummy implementation
     }
@@ -1042,19 +1132,27 @@ public class ContainerLaunch implements Callable<Integer> {
     protected abstract void link(Path src, Path dst) throws IOException;
 
     protected abstract void mkdir(Path path) throws IOException;
+
+    boolean doRedirectStdOut() {
+      return redirectStdOut;
+    }
+
+    boolean doRedirectStdErr() {
+      return redirectStdErr;
+    }
+
   }
 
   private static final class UnixShellScriptBuilder extends ShellScriptBuilder {
-
     private void errorCheck() {
       line("hadoop_shell_errorcode=$?");
-      line("if [ $hadoop_shell_errorcode -ne 0 ]");
+      line("if [[ \"$hadoop_shell_errorcode\" -ne 0 ]]");
       line("then");
       line("  exit $hadoop_shell_errorcode");
       line("fi");
     }
 
-    public UnixShellScriptBuilder(){
+    public UnixShellScriptBuilder() {
       line("#!/bin/bash");
       line();
     }
@@ -1062,29 +1160,47 @@ public class ContainerLaunch implements Callable<Integer> {
     @Override
     public void command(List<String> command) {
       line("exec /bin/bash -c \"", StringUtils.join(" ", command), "\"");
-      errorCheck();
     }
 
     @Override
-    public void whitelistedEnv(String key, String value) {
+    public void whitelistedEnv(String key, String value) throws IOException {
       line("export ", key, "=${", key, ":-", "\"", value, "\"}");
     }
 
     @Override
-    public void env(String key, String value) {
+    public void setStdOut(final Path stdout) throws IOException {
+      line("export ", ENV_PRELAUNCH_STDOUT, "=\"", stdout.toString(), "\"");
+      // tee is needed for DefaultContainerExecutor error propagation to stdout
+      // Close stdout of subprocess to prevent it from writing to the stdout file
+      line("exec >\"${" + ENV_PRELAUNCH_STDOUT + "}\"");
+    }
+
+    @Override
+    public void setStdErr(final Path stderr) throws IOException {
+      line("export ", ENV_PRELAUNCH_STDERR, "=\"", stderr.toString(), "\"");
+      // tee is needed for DefaultContainerExecutor error propagation to stderr
+      // Close stdout of subprocess to prevent it from writing to the stdout file
+      line("exec 2>\"${" + ENV_PRELAUNCH_STDERR + "}\"");
+    }
+
+    @Override
+    public void env(String key, String value) throws IOException {
       line("export ", key, "=\"", value, "\"");
     }
 
     @Override
+    public void echo(final String echoStr) throws IOException {
+      line("echo \"" + echoStr + "\"");
+    }
+
+    @Override
     protected void link(Path src, Path dst) throws IOException {
       line("ln -sf \"", src.toUri().getPath(), "\" \"", dst.toString(), "\"");
-      errorCheck();
     }
 
     @Override
-    protected void mkdir(Path path) {
+    protected void mkdir(Path path) throws IOException {
       line("mkdir -p ", path.toString());
-      errorCheck();
     }
 
     @Override
@@ -1152,6 +1268,16 @@ public class ContainerLaunch implements Callable<Integer> {
       errorCheck();
     }
 
+    //Dummy implementation
+    @Override
+    protected void setStdOut(final Path stdout) throws IOException {
+    }
+
+    //Dummy implementation
+    @Override
+    protected void setStdErr(final Path stderr) throws IOException {
+    }
+
     @Override
     public void env(String key, String value) throws IOException {
       lineWithLenCheck("@set ", key, "=", value);
@@ -1159,6 +1285,11 @@ public class ContainerLaunch implements Callable<Integer> {
     }
 
     @Override
+    public void echo(final String echoStr) throws IOException {
+      lineWithLenCheck("@echo \"", echoStr, "\"");
+    }
+
+    @Override
     protected void link(Path src, Path dst) throws IOException {
       File srcFile = new File(src.toUri().getPath());
       String srcFileStr = srcFile.getPath();

http://git-wip-us.apache.org/repos/asf/hadoop/blob/febeead5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
index 7176942..588fdcd 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
@@ -32,6 +32,7 @@ import java.io.IOException;
 import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -192,7 +193,11 @@ public class TestContainerLaunch extends BaseContainerManagerTest {
 
       shexc.execute();
       assertEquals(shexc.getExitCode(), 0);
-      assert(shexc.getOutput().contains("hello"));
+      //Capture output from prelaunch.out
+
+      List<String> output = Files.readAllLines(Paths.get(localLogDir.getAbsolutePath(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT),
+          Charset.forName("UTF-8"));
+      assert(output.contains("hello"));
 
       symLinkFile = new File(tmpDir, badSymlink);
     }
@@ -358,7 +363,10 @@ public class TestContainerLaunch extends BaseContainerManagerTest {
         shexc.execute();
         Assert.fail("Should catch exception");
       } catch(ExitCodeException e){
-        diagnostics = e.getMessage();
+        //Capture diagnostics from prelaunch.stderr
+        List<String> error = Files.readAllLines(Paths.get(localLogDir.getAbsolutePath(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR),
+            Charset.forName("UTF-8"));
+        diagnostics = StringUtils.join("\n", error);
       }
       Assert.assertTrue(diagnostics.contains(Shell.WINDOWS ?
           "is not recognized as an internal or external command" :
@@ -1545,6 +1553,109 @@ public class TestContainerLaunch extends BaseContainerManagerTest {
 
   }
 
+  /**
+   * Test that script exists with non-zero exit code when command fails.
+   * @throws IOException
+   */
+  @Test
+  public void testShellScriptBuilderStdOutandErrRedirection() throws IOException {
+    ShellScriptBuilder builder = ShellScriptBuilder.create();
+
+    Path logDir = new Path(localLogDir.getAbsolutePath());
+    File stdout = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT);
+    File stderr = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR);
+
+    builder.stdout(logDir, ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT);
+    builder.stderr(logDir, ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR);
+
+    //should redirect to specified stdout path
+    String TEST_STDOUT_ECHO = "Test stdout redirection";
+    builder.echo(TEST_STDOUT_ECHO);
+    //should fail and redirect to stderr
+    builder.mkdir(new Path("/invalidSrcDir"));
+
+    builder.command(Arrays.asList(new String[] {"unknownCommand"}));
+
+    File shellFile = Shell.appendScriptExtension(tmpDir, "testShellScriptBuilderStdOutandErrRedirection");
+    PrintStream writer = new PrintStream(new FileOutputStream(shellFile));
+    builder.write(writer);
+    writer.close();
+    try {
+      FileUtil.setExecutable(shellFile, true);
+
+      Shell.ShellCommandExecutor shexc = new Shell.ShellCommandExecutor(
+          new String[]{shellFile.getAbsolutePath()}, tmpDir);
+      try {
+        shexc.execute();
+        fail("builder shell command was expected to throw");
+      }
+      catch(IOException e) {
+        // expected
+        System.out.println("Received an expected exception: " + e.getMessage());
+
+        Assert.assertEquals(true, stdout.exists());
+        BufferedReader stdoutReader = new BufferedReader(new FileReader(stdout));
+        // Get the pid of the process
+        String line = stdoutReader.readLine().trim();
+        Assert.assertEquals(TEST_STDOUT_ECHO, line);
+        // No more lines
+        Assert.assertEquals(null, stdoutReader.readLine());
+        stdoutReader.close();
+
+        Assert.assertEquals(true, stderr.exists());
+        Assert.assertTrue(stderr.length() > 0);
+      }
+    }
+    finally {
+      FileUtil.fullyDelete(shellFile);
+      FileUtil.fullyDelete(stdout);
+      FileUtil.fullyDelete(stderr);
+    }
+  }
+
+  /**
+   * Test that script exists with non-zero exit code when command fails.
+   * @throws IOException
+   */
+  @Test
+  public void testShellScriptBuilderWithNoRedirection() throws IOException {
+    ShellScriptBuilder builder = ShellScriptBuilder.create();
+
+    Path logDir = new Path(localLogDir.getAbsolutePath());
+    File stdout = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT);
+    File stderr = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR);
+
+    //should redirect to specified stdout path
+    String TEST_STDOUT_ECHO = "Test stdout redirection";
+    builder.echo(TEST_STDOUT_ECHO);
+    //should fail and redirect to stderr
+    builder.mkdir(new Path("/invalidSrcDir"));
+
+    builder.command(Arrays.asList(new String[]{"unknownCommand"}));
+
+    File shellFile = Shell.appendScriptExtension(tmpDir, "testShellScriptBuilderStdOutandErrRedirection");
+    PrintStream writer = new PrintStream(new FileOutputStream(shellFile));
+    builder.write(writer);
+    writer.close();
+    try {
+      FileUtil.setExecutable(shellFile, true);
+
+      Shell.ShellCommandExecutor shexc = new Shell.ShellCommandExecutor(
+          new String[]{shellFile.getAbsolutePath()}, tmpDir);
+      try {
+        shexc.execute();
+        fail("builder shell command was expected to throw");
+      } catch (IOException e) {
+        // expected
+        System.out.println("Received an expected exception: " + e.getMessage());
+
+        Assert.assertEquals(false, stdout.exists());
+        Assert.assertEquals(false, stderr.exists());
+      }
+    } finally {
+      FileUtil.fullyDelete(shellFile);
+    }
+  }
   /*
    * ${foo.version} is substituted to suffix a specific version number
    */


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


[2/2] hadoop git commit: YARN-6509. Add a size threshold beyond which yarn logs will require a force option. (Xuan Gong via wangda)

Posted by wa...@apache.org.
YARN-6509. Add a size threshold beyond which yarn logs will require a force option. (Xuan Gong via wangda)

Change-Id: I755fe903337d4ff9ec35dae5b9cce638794e1d0f


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/ec2ae306
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/ec2ae306
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/ec2ae306

Branch: refs/heads/trunk
Commit: ec2ae3060a807c8754826af2135a68c08b2e4f13
Parents: febeead
Author: Wangda Tan <wa...@apache.org>
Authored: Fri Sep 29 16:42:56 2017 -0700
Committer: Wangda Tan <wa...@apache.org>
Committed: Fri Sep 29 16:42:56 2017 -0700

----------------------------------------------------------------------
 .../apache/hadoop/yarn/client/cli/LogsCLI.java  | 366 ++++++++++++-------
 .../hadoop/yarn/client/cli/TestLogsCLI.java     |  41 ++-
 .../yarn/logaggregation/LogCLIHelpers.java      |   8 +-
 .../tfile/LogAggregationTFileController.java    |   6 +
 4 files changed, 279 insertions(+), 142 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/ec2ae306/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java
index 9a8ba4a..74497ce 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java
@@ -29,9 +29,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.regex.Pattern;
@@ -107,6 +110,7 @@ public class LogsCLI extends Configured implements Tool {
   private static final String CLIENT_RETRY_INTERVAL_OPTION
       = "client_retry_interval_ms";
   public static final String HELP_CMD = "help";
+  private static final String SIZE_LIMIT_OPTION = "size_limit_mb";
 
   private PrintStream outStream = System.out;
   private YarnClient yarnClient = null;
@@ -115,6 +119,11 @@ public class LogsCLI extends Configured implements Tool {
   private static final int DEFAULT_MAX_RETRIES = 30;
   private static final long DEFAULT_RETRY_INTERVAL = 1000;
 
+  private static final long LOG_SIZE_LIMIT_DEFAULT = 10240L;
+
+  private long logSizeLeft = LOG_SIZE_LIMIT_DEFAULT * 1024 * 1024;
+  private long specifedLogLimits = LOG_SIZE_LIMIT_DEFAULT;
+
   @Private
   @VisibleForTesting
   ClientConnectionRetry connectionRetry;
@@ -158,6 +167,7 @@ public class LogsCLI extends Configured implements Tool {
     List<String> amContainersList = new ArrayList<String>();
     String localDir = null;
     long bytes = Long.MAX_VALUE;
+    boolean ignoreSizeLimit = false;
     int maxRetries = DEFAULT_MAX_RETRIES;
     long retryInterval = DEFAULT_RETRY_INTERVAL;
     try {
@@ -199,6 +209,14 @@ public class LogsCLI extends Configured implements Tool {
         retryInterval = Long.parseLong(commandLine.getOptionValue(
             CLIENT_RETRY_INTERVAL_OPTION));
       }
+      if (commandLine.hasOption(SIZE_LIMIT_OPTION)) {
+        specifedLogLimits = Long.parseLong(commandLine.getOptionValue(
+            SIZE_LIMIT_OPTION));
+        logSizeLeft = specifedLogLimits * 1024 * 1024;
+      }
+      if (logSizeLeft < 0L) {
+        ignoreSizeLimit = true;
+      }
     } catch (ParseException e) {
       System.err.println("options parsing failed: " + e.getMessage());
       printHelpMessage(printOpts);
@@ -306,6 +324,7 @@ public class LogsCLI extends Configured implements Tool {
       logs.addAll(Arrays.asList(logFilesRegex));
     }
 
+
     ContainerLogsRequest request = new ContainerLogsRequest(appId,
         isApplicationFinished(appState), appOwner, nodeAddress, null,
         containerIdStr, localDir, logs, bytes, null);
@@ -324,15 +343,17 @@ public class LogsCLI extends Configured implements Tool {
     // To get am logs
     if (getAMContainerLogs) {
       return fetchAMContainerLogs(request, amContainersList,
-          logCliHelper, useRegex);
+          logCliHelper, useRegex, ignoreSizeLimit);
     }
 
     int resultCode = 0;
     if (containerIdStr != null) {
-      return fetchContainerLogs(request, logCliHelper, useRegex);
+      return fetchContainerLogs(request, logCliHelper, useRegex,
+          ignoreSizeLimit);
     } else {
       if (nodeAddress == null) {
-        resultCode = fetchApplicationLogs(request, logCliHelper, useRegex);
+        resultCode = fetchApplicationLogs(request, logCliHelper, useRegex,
+            ignoreSizeLimit);
       } else {
         System.err.println("Should at least provide ContainerId!");
         printHelpMessage(printOpts);
@@ -524,35 +545,16 @@ public class LogsCLI extends Configured implements Tool {
   @VisibleForTesting
   public int printContainerLogsFromRunningApplication(Configuration conf,
       ContainerLogsRequest request, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws IOException {
+      boolean useRegex, boolean ignoreSizeLimit) throws IOException {
     String containerIdStr = request.getContainerId().toString();
     String localDir = request.getOutputLocalDir();
-    String nodeHttpAddress = request.getNodeHttpAddress();
-    if (nodeHttpAddress == null || nodeHttpAddress.isEmpty()) {
-      System.err.println("Can not get the logs for the container: "
-          + containerIdStr);
-      System.err.println("The node http address is required to get container "
-          + "logs for the Running application.");
-      return -1;
-    }
     String nodeId = request.getNodeId();
     PrintStream out = LogToolUtils.createPrintStream(localDir, nodeId,
         containerIdStr);
     try {
-      Set<String> matchedFiles = getMatchedContainerLogFiles(request,
-          useRegex);
-      if (matchedFiles.isEmpty()) {
-        System.err.println("Can not find any log file matching the pattern: "
-            + request.getLogTypes() + " for the container: " + containerIdStr
-            + " within the application: " + request.getAppId());
-        return -1;
-      }
-      ContainerLogsRequest newOptions = new ContainerLogsRequest(request);
-      newOptions.setLogTypes(matchedFiles);
-
       boolean foundAnyLogs = false;
       byte[] buffer = new byte[65536];
-      for (String logFile : newOptions.getLogTypes()) {
+      for (String logFile : request.getLogTypes()) {
         InputStream is = null;
         try {
           ClientResponse response = getResponeFromNMWebService(conf,
@@ -595,50 +597,6 @@ public class LogsCLI extends Configured implements Tool {
     }
   }
 
-  private int printContainerLogsForFinishedApplication(
-      ContainerLogsRequest request, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws IOException {
-    ContainerLogsRequest newOptions = getMatchedLogOptions(
-        request, logCliHelper, useRegex);
-    if (newOptions == null) {
-      System.err.println("Can not find any log file matching the pattern: "
-          + request.getLogTypes() + " for the container: "
-          + request.getContainerId() + " within the application: "
-          + request.getAppId());
-      return -1;
-    }
-    return logCliHelper.dumpAContainerLogsForLogType(newOptions);
-  }
-
-  private int printContainerLogsForFinishedApplicationWithoutNodeId(
-      ContainerLogsRequest request, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws IOException {
-    ContainerLogsRequest newOptions = getMatchedLogOptions(
-        request, logCliHelper, useRegex);
-    if (newOptions == null) {
-      System.err.println("Can not find any log file matching the pattern: "
-          + request.getLogTypes() + " for the container: "
-          + request.getContainerId() + " within the application: "
-          + request.getAppId());
-      return -1;
-    }
-    return logCliHelper.dumpAContainerLogsForLogTypeWithoutNodeId(
-        newOptions);
-  }
-
-  private int printAggregatedContainerLogs(ContainerLogsRequest request,
-      LogCLIHelpers logCliHelper, boolean useRegex) throws IOException {
-    return printContainerLogsForFinishedApplication(request,
-        logCliHelper, useRegex);
-  }
-
-  private int printAggregatedContainerLogsWithoutNodeId(
-      ContainerLogsRequest request, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws IOException {
-    return printContainerLogsForFinishedApplicationWithoutNodeId(request,
-        logCliHelper, useRegex);
-  }
-
   @Private
   @VisibleForTesting
   public ContainerReport getContainerReport(String containerIdStr)
@@ -655,7 +613,8 @@ public class LogsCLI extends Configured implements Tool {
 
   private int printAMContainerLogs(Configuration conf,
       ContainerLogsRequest request, List<String> amContainers,
-      LogCLIHelpers logCliHelper, boolean useRegex) throws Exception {
+      LogCLIHelpers logCliHelper, boolean useRegex, boolean ignoreSizeLimit)
+      throws Exception {
     List<JSONObject> amContainersList = null;
     List<ContainerLogsRequest> requests =
         new ArrayList<ContainerLogsRequest>();
@@ -717,10 +676,9 @@ public class LogsCLI extends Configured implements Tool {
       return -1;
     }
 
+    List<ContainerLogsRequest> candidates = new ArrayList<>();
     if (amContainers.contains("ALL")) {
-      for (ContainerLogsRequest amRequest : requests) {
-        outputAMContainerLogs(amRequest, conf, logCliHelper, useRegex);
-      }
+      candidates.addAll(requests);
       outStream.println();
       outStream.println("Specified ALL for -am option. "
           + "Printed logs for all am containers.");
@@ -728,12 +686,10 @@ public class LogsCLI extends Configured implements Tool {
       for (String amContainer : amContainers) {
         int amContainerId = Integer.parseInt(amContainer.trim());
         if (amContainerId == -1) {
-          outputAMContainerLogs(requests.get(requests.size() - 1), conf,
-              logCliHelper, useRegex);
+          candidates.add(requests.get(requests.size() - 1));
         } else {
           if (amContainerId <= requests.size()) {
-            outputAMContainerLogs(requests.get(amContainerId - 1), conf,
-                logCliHelper, useRegex);
+            candidates.add(requests.get(amContainerId - 1));
           } else {
             System.err.println(String.format("ERROR: Specified AM containerId"
                 + " (%s) exceeds the number of AM containers (%s).",
@@ -743,12 +699,25 @@ public class LogsCLI extends Configured implements Tool {
         }
       }
     }
+    Map<String, ContainerLogsRequest> newOptions = new HashMap<>();
+    if (request.isAppFinished()) {
+      newOptions = getMatchedLogTypesForFinishedApp(candidates,
+          logCliHelper, useRegex, ignoreSizeLimit);
+    } else {
+      newOptions = getMatchedLogTypesForRunningApp(candidates, useRegex,
+          ignoreSizeLimit);
+    }
+    for (Entry<String, ContainerLogsRequest> amRequest
+        : newOptions.entrySet()) {
+      outputAMContainerLogs(amRequest.getValue(), conf, logCliHelper,
+          useRegex, ignoreSizeLimit);
+    }
     return 0;
   }
 
   private void outputAMContainerLogs(ContainerLogsRequest request,
-      Configuration conf, LogCLIHelpers logCliHelper, boolean useRegex)
-      throws Exception {
+      Configuration conf, LogCLIHelpers logCliHelper, boolean useRegex,
+      boolean ignoreSizeLimit) throws Exception {
     String nodeHttpAddress = request.getNodeHttpAddress();
     String containerId = request.getContainerId();
     String nodeId = request.getNodeId();
@@ -756,11 +725,10 @@ public class LogsCLI extends Configured implements Tool {
     if (request.isAppFinished()) {
       if (containerId != null && !containerId.isEmpty()) {
         if (nodeId != null && !nodeId.isEmpty()) {
-          printContainerLogsForFinishedApplication(request,
-              logCliHelper, useRegex);
+          logCliHelper.dumpAContainerLogsForLogType(request);
         } else {
-          printContainerLogsForFinishedApplicationWithoutNodeId(
-              request, logCliHelper, useRegex);
+          logCliHelper.dumpAContainerLogsForLogTypeWithoutNodeId(
+              request);
         }
       }
     } else {
@@ -770,7 +738,7 @@ public class LogsCLI extends Configured implements Tool {
             .getContainerState();
         request.setContainerState(containerState);
         printContainerLogsFromRunningApplication(conf,
-            request, logCliHelper, useRegex);
+            request, logCliHelper, useRegex, ignoreSizeLimit);
       }
     }
   }
@@ -898,6 +866,13 @@ public class LogsCLI extends Configured implements Tool {
     opts.addOption(CLIENT_RETRY_INTERVAL_OPTION, true,
         "Work with --client_max_retries to create a retry client. "
         + "The default value is 1000.");
+    opts.addOption(SIZE_LIMIT_OPTION, true, "Use this option to limit "
+        + "the size of the total logs which could be fetched. "
+        + "By default, we only allow to fetch at most "
+        + LOG_SIZE_LIMIT_DEFAULT + " MB logs. If the total log size is "
+        + "larger than the specified number, the CLI would fail. "
+        + "The user could specify -1 to ignore the size limit "
+        + "and fetch all logs.");
     opts.getOption(APPLICATION_ID_OPTION).setArgName("Application ID");
     opts.getOption(CONTAINER_ID_OPTION).setArgName("Container ID");
     opts.getOption(NODE_ADDRESS_OPTION).setArgName("Node Address");
@@ -908,6 +883,7 @@ public class LogsCLI extends Configured implements Tool {
     opts.getOption(CLIENT_MAX_RETRY_OPTION).setArgName("Max Retries");
     opts.getOption(CLIENT_RETRY_INTERVAL_OPTION)
         .setArgName("Retry Interval");
+    opts.getOption(SIZE_LIMIT_OPTION).setArgName("Size Limit");
     return opts;
   }
 
@@ -933,6 +909,7 @@ public class LogsCLI extends Configured implements Tool {
         PER_CONTAINER_LOG_FILES_REGEX_OPTION));
     printOpts.addOption(commandOpts.getOption(CLIENT_MAX_RETRY_OPTION));
     printOpts.addOption(commandOpts.getOption(CLIENT_RETRY_INTERVAL_OPTION));
+    printOpts.addOption(commandOpts.getOption(SIZE_LIMIT_OPTION));
     return printOpts;
   }
 
@@ -969,15 +946,15 @@ public class LogsCLI extends Configured implements Tool {
 
   private int fetchAMContainerLogs(ContainerLogsRequest request,
       List<String> amContainersList, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws Exception {
+      boolean useRegex, boolean ignoreSizeLimit) throws Exception {
     return printAMContainerLogs(getConf(), request, amContainersList,
-        logCliHelper, useRegex);
+        logCliHelper, useRegex, ignoreSizeLimit);
   }
 
   private int fetchContainerLogs(ContainerLogsRequest request,
-      LogCLIHelpers logCliHelper, boolean useRegex) throws IOException,
-      ClientHandlerException, UniformInterfaceException, JSONException {
-    int resultCode = 0;
+      LogCLIHelpers logCliHelper, boolean useRegex, boolean ignoreSizeLimit)
+      throws IOException, ClientHandlerException, UniformInterfaceException,
+      JSONException {
     String appIdStr = request.getAppId().toString();
     String containerIdStr = request.getContainerId();
     String nodeAddress = request.getNodeId();
@@ -988,12 +965,20 @@ public class LogsCLI extends Configured implements Tool {
     if (isAppFinished) {
       // if user specified "ALL" as the logFiles param, pass empty list
       // to logCliHelper so that it fetches all the logs
+      ContainerLogsRequest newOptions = getMatchedLogOptions(
+          request, logCliHelper, useRegex, ignoreSizeLimit);
+      if (newOptions == null) {
+        System.err.println("Can not find any log file matching the pattern: "
+            + request.getLogTypes() + " for the container: "
+            + request.getContainerId() + " within the application: "
+            + request.getAppId());
+        return -1;
+      }
       if (nodeAddress != null && !nodeAddress.isEmpty()) {
-        return printContainerLogsForFinishedApplication(
-            request, logCliHelper, useRegex);
+        return logCliHelper.dumpAContainerLogsForLogType(newOptions);
       } else {
-        return printContainerLogsForFinishedApplicationWithoutNodeId(
-            request, logCliHelper, useRegex);
+        return logCliHelper.dumpAContainerLogsForLogTypeWithoutNodeId(
+            newOptions);
       }
     }
     String nodeHttpAddress = null;
@@ -1019,13 +1004,20 @@ public class LogsCLI extends Configured implements Tool {
       } else {
         // for the case, we have already uploaded partial logs in HDFS
         int result = -1;
-        if (nodeAddress != null && !nodeAddress.isEmpty()) {
-          result = printAggregatedContainerLogs(request,
-              logCliHelper, useRegex);
+        ContainerLogsRequest newOptions = getMatchedLogOptions(
+                request, logCliHelper, useRegex, ignoreSizeLimit);
+        if (newOptions == null) {
+          System.err.println("Can not find any log file matching the pattern: "
+              + request.getLogTypes() + " for the container: "
+              + request.getContainerId() + " within the application: "
+              + request.getAppId());
         } else {
-          result = printAggregatedContainerLogsWithoutNodeId(request,
-              logCliHelper,
-                  useRegex);
+          if (nodeAddress != null && !nodeAddress.isEmpty()) {
+            result = logCliHelper.dumpAContainerLogsForLogType(newOptions);
+          } else {
+            result = logCliHelper.dumpAContainerLogsForLogTypeWithoutNodeId(
+                newOptions);
+          }
         }
         if (result == -1) {
           System.err.println(
@@ -1043,14 +1035,18 @@ public class LogsCLI extends Configured implements Tool {
     // If the application is not in the final state,
     // we will provide the NodeHttpAddress and get the container logs
     // by calling NodeManager webservice.
-    resultCode = printContainerLogsFromRunningApplication(getConf(), request,
-        logCliHelper, useRegex);
-    return resultCode;
+    ContainerLogsRequest newRequest = getMatchedOptionForRunningApp(
+        request, useRegex, ignoreSizeLimit);
+    if (newRequest == null) {
+      return -1;
+    }
+    return printContainerLogsFromRunningApplication(getConf(), request,
+          logCliHelper, useRegex, ignoreSizeLimit);
   }
 
   private int fetchApplicationLogs(ContainerLogsRequest options,
-      LogCLIHelpers logCliHelper, boolean useRegex) throws IOException,
-      YarnException {
+      LogCLIHelpers logCliHelper, boolean useRegex, boolean ignoreSizeLimit)
+      throws IOException, YarnException {
     // If the application has finished, we would fetch the logs
     // from HDFS.
     // If the application is still running, we would get the full
@@ -1059,7 +1055,7 @@ public class LogsCLI extends Configured implements Tool {
     int resultCode = -1;
     if (options.isAppFinished()) {
       ContainerLogsRequest newOptions = getMatchedLogOptions(
-          options, logCliHelper, useRegex);
+          options, logCliHelper, useRegex, ignoreSizeLimit);
       if (newOptions == null) {
         System.err.println("Can not find any log file matching the pattern: "
             + options.getLogTypes() + " for the application: "
@@ -1071,9 +1067,17 @@ public class LogsCLI extends Configured implements Tool {
     } else {
       List<ContainerLogsRequest> containerLogRequests =
           getContainersLogRequestForRunningApplication(options);
-      for (ContainerLogsRequest container : containerLogRequests) {
+
+      // get all matched container log types and check the total log size.
+      Map<String, ContainerLogsRequest> matchedLogTypes =
+          getMatchedLogTypesForRunningApp(containerLogRequests,
+              useRegex, ignoreSizeLimit);
+
+      for (Entry<String, ContainerLogsRequest> container
+          : matchedLogTypes.entrySet()) {
         int result = printContainerLogsFromRunningApplication(getConf(),
-            container, logCliHelper, useRegex);
+            container.getValue(), logCliHelper,
+            useRegex, ignoreSizeLimit);
         if (result == 0) {
           resultCode = 0;
         }
@@ -1103,37 +1107,54 @@ public class LogsCLI extends Configured implements Tool {
 
   private ContainerLogsRequest getMatchedLogOptions(
       ContainerLogsRequest request, LogCLIHelpers logCliHelper,
-      boolean useRegex) throws IOException {
+      boolean useRegex, boolean ignoreSizeLimit) throws IOException {
     ContainerLogsRequest newOptions = new ContainerLogsRequest(request);
-    if (request.getLogTypes() != null && !request.getLogTypes().isEmpty()) {
-      Set<String> matchedFiles = new HashSet<String>();
-      if (!request.getLogTypes().contains("ALL")) {
-        Set<String> files = logCliHelper.listContainerLogs(request);
-        matchedFiles = getMatchedLogFiles(request, files, useRegex);
-        if (matchedFiles.isEmpty()) {
-          return null;
-        }
-      }
+    Set<ContainerLogFileInfo> files = logCliHelper.listContainerLogs(
+        request);
+    Set<String> matchedFiles = getMatchedLogFiles(request, files,
+        useRegex, ignoreSizeLimit);
+    if (matchedFiles.isEmpty()) {
+      return null;
+    } else {
       newOptions.setLogTypes(matchedFiles);
+      return newOptions;
     }
-    return newOptions;
   }
 
   private Set<String> getMatchedLogFiles(ContainerLogsRequest options,
-      Collection<String> candidate, boolean useRegex) throws IOException {
+      Collection<ContainerLogFileInfo> candidate, boolean useRegex,
+      boolean ignoreSizeLimit) throws IOException {
     Set<String> matchedFiles = new HashSet<String>();
     Set<String> filePattern = options.getLogTypes();
-    if (options.getLogTypes().contains("ALL")) {
-      return new HashSet<String>(candidate);
-    }
-    for (String file : candidate) {
-      if (useRegex) {
-        if (isFileMatching(file, filePattern)) {
-          matchedFiles.add(file);
+    long size = options.getBytes();
+    boolean getAll = options.getLogTypes().contains("ALL");
+    Iterator<ContainerLogFileInfo> iterator = candidate.iterator();
+    while(iterator.hasNext()) {
+      boolean matchedFile = false;
+      ContainerLogFileInfo logInfo = iterator.next();
+      if (getAll) {
+        matchedFile = true;
+      } else if (useRegex) {
+        if (isFileMatching(logInfo.getFileName(), filePattern)) {
+          matchedFile = true;
         }
       } else {
-        if (filePattern.contains(file)) {
-          matchedFiles.add(file);
+        if (filePattern.contains(logInfo.getFileName())) {
+          matchedFile = true;
+        }
+      }
+      if (matchedFile) {
+        matchedFiles.add(logInfo.getFileName());
+        if (!ignoreSizeLimit) {
+          decrLogSizeLimit(Math.min(
+              Long.parseLong(logInfo.getFileSize()), size));
+          if (getLogSizeLimitLeft() < 0) {
+            throw new RuntimeException("The total log size is too large."
+                + "The log size limit is " + specifedLogLimits + "MB. "
+                + "Please specify a proper value --size option or if you "
+                + "really want to fetch all, please "
+                + "specify -1 for --size_limit_mb option.");
+          }
         }
       }
     }
@@ -1296,18 +1317,19 @@ public class LogsCLI extends Configured implements Tool {
 
   @VisibleForTesting
   public Set<String> getMatchedContainerLogFiles(ContainerLogsRequest request,
-      boolean useRegex) throws IOException {
+      boolean useRegex, boolean ignoreSizeLimit) throws IOException {
     // fetch all the log files for the container
     // filter the log files based on the given -log_files pattern
     List<Pair<ContainerLogFileInfo, String>> allLogFileInfos=
         getContainerLogFiles(getConf(), request.getContainerId(),
             request.getNodeHttpAddress());
-    List<String> fileNames = new ArrayList<String>();
+    List<ContainerLogFileInfo> fileNames = new ArrayList<
+        ContainerLogFileInfo>();
     for (Pair<ContainerLogFileInfo, String> fileInfo : allLogFileInfos) {
-      fileNames.add(fileInfo.getKey().getFileName());
+      fileNames.add(fileInfo.getKey());
     }
     return getMatchedLogFiles(request, fileNames,
-        useRegex);
+        useRegex, ignoreSizeLimit);
   }
 
   @VisibleForTesting
@@ -1451,4 +1473,86 @@ public class LogsCLI extends Configured implements Tool {
     // The method to indicate if we should retry given the incoming exception
     public abstract boolean shouldRetryOn(Exception e);
   }
+
+  private long getLogSizeLimitLeft() {
+    return this.logSizeLeft;
+  }
+
+  private void decrLogSizeLimit(long used) {
+    this.logSizeLeft -= used;
+  }
+
+  @Private
+  @VisibleForTesting
+  public ContainerLogsRequest getMatchedOptionForRunningApp(
+      ContainerLogsRequest container, boolean useRegex,
+      boolean ignoreSizeLimit) throws IOException {
+    String containerIdStr = container.getContainerId().toString();
+    String nodeHttpAddress = container.getNodeHttpAddress();
+    if (nodeHttpAddress == null || nodeHttpAddress.isEmpty()) {
+      System.err.println("Can not get the logs for the container: "
+          + containerIdStr);
+      System.err.println("The node http address is required to get container "
+          + "logs for the Running application.");
+      return null;
+    }
+
+    Set<String> matchedFiles = getMatchedContainerLogFiles(container,
+        useRegex, ignoreSizeLimit);
+    if (matchedFiles.isEmpty()) {
+      System.err.println("Can not find any log file matching the pattern: "
+          + container.getLogTypes() + " for the container: " + containerIdStr
+          + " within the application: " + container.getAppId());
+      return null;
+    }
+    container.setLogTypes(matchedFiles);
+    return container;
+  }
+
+  @Private
+  @VisibleForTesting
+  public Map<String, ContainerLogsRequest> getMatchedLogTypesForRunningApp(
+      List<ContainerLogsRequest> containerLogRequests, boolean useRegex,
+      boolean ignoreSizeLimit) {
+    Map<String, ContainerLogsRequest> containerMatchedLog = new HashMap<>();
+    for (ContainerLogsRequest container : containerLogRequests) {
+      try {
+        ContainerLogsRequest request = getMatchedOptionForRunningApp(
+            container, useRegex, ignoreSizeLimit);
+        if (request == null) {
+          continue;
+        }
+        containerMatchedLog.put(container.getContainerId(), request);
+      } catch(IOException ex) {
+        System.err.println(ex);
+        continue;
+      }
+    }
+    return containerMatchedLog;
+  }
+
+  private Map<String, ContainerLogsRequest> getMatchedLogTypesForFinishedApp(
+      List<ContainerLogsRequest> containerLogRequests,
+      LogCLIHelpers logCliHelper, boolean useRegex,
+      boolean ignoreSizeLimit) {
+    Map<String, ContainerLogsRequest> containerMatchedLog = new HashMap<>();
+    for (ContainerLogsRequest container : containerLogRequests) {
+      try {
+        ContainerLogsRequest request = getMatchedLogOptions(container,
+            logCliHelper, useRegex, ignoreSizeLimit);
+        if (request == null) {
+          System.err.println("Can not find any log file matching the pattern: "
+              + container.getLogTypes() + " for the container: "
+              + container.getContainerId() + " within the application: "
+              + container.getAppId());
+          continue;
+        }
+        containerMatchedLog.put(container.getContainerId(), request);
+      } catch (IOException ex) {
+        System.err.println(ex);
+        continue;
+      }
+    }
+    return containerMatchedLog;
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/ec2ae306/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java
index fed7488..c292430 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java
@@ -30,6 +30,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Matchers.*;
 
 import com.sun.jersey.api.client.Client;
 import com.sun.jersey.api.client.ClientResponse;
@@ -300,6 +301,17 @@ public class TestLogsCLI {
     pw.println("                                              end and positive values as");
     pw.println("                                              bytes to read from the");
     pw.println("                                              beginning.");
+    pw.println(" -size_limit_mb <Size Limit>                  Use this option to limit the");
+    pw.println("                                              size of the total logs which");
+    pw.println("                                              could be fetched. By");
+    pw.println("                                              default, we only allow to");
+    pw.println("                                              fetch at most 10240 MB logs.");
+    pw.println("                                              If the total log size is");
+    pw.println("                                              larger than the specified");
+    pw.println("                                              number, the CLI would fail.");
+    pw.println("                                              The user could specify -1 to");
+    pw.println("                                              ignore the size limit and");
+    pw.println("                                              fetch all logs.");
     pw.close();
     String appReportStr = baos.toString("UTF-8");
     Assert.assertTrue(sysOutStream.toString().contains(appReportStr));
@@ -563,8 +575,7 @@ public class TestLogsCLI {
             containerId0.toString() });
     assertTrue(exitCode == -1);
     assertTrue(sysErrStream.toString().contains(
-      "Logs for container " + containerId0.toString()
-          + " are not present in this log-file."));
+        "Can not find any log file matching the pattern"));
     sysErrStream.reset();
 
     // uploaded two logs for container3. The first log is named as syslog.
@@ -750,7 +761,7 @@ public class TestLogsCLI {
       Set<String> logsSet = new HashSet<String>();
       logsSet.add(fileName);
       doReturn(logsSet).when(cli).getMatchedContainerLogFiles(
-          any(ContainerLogsRequest.class), anyBoolean());
+          any(ContainerLogsRequest.class), anyBoolean(), anyBoolean());
       ClientResponse mockReponse = mock(ClientResponse.class);
       doReturn(Status.OK).when(mockReponse).getStatusInfo();
       doReturn(fis).when(mockReponse).getEntityInputStream();
@@ -795,6 +806,7 @@ public class TestLogsCLI {
     doReturn(nodeId).when(mockContainerReport1).getAssignedNode();
     doReturn("http://localhost:2345").when(mockContainerReport1)
         .getNodeHttpAddress();
+
     ContainerId containerId2 = ContainerId.newContainerId(appAttemptId, 2);
     ContainerReport mockContainerReport2 = mock(ContainerReport.class);
     doReturn(containerId2).when(mockContainerReport2).getContainerId();
@@ -812,7 +824,19 @@ public class TestLogsCLI {
     LogsCLI cli = spy(new LogsCLIForTest(mockYarnClient));
     doReturn(0).when(cli).printContainerLogsFromRunningApplication(
         any(Configuration.class), any(ContainerLogsRequest.class),
-        any(LogCLIHelpers.class), anyBoolean());
+        any(LogCLIHelpers.class), anyBoolean(), anyBoolean());
+    Set<String> logTypes = new HashSet<>();
+    logTypes.add("ALL");
+    ContainerLogsRequest mockContainer1 = mock(ContainerLogsRequest.class);
+    doReturn(logTypes).when(mockContainer1).getLogTypes();
+    ContainerLogsRequest mockContainer2 = mock(ContainerLogsRequest.class);
+    doReturn(logTypes).when(mockContainer2).getLogTypes();
+    Map<String, ContainerLogsRequest> matchedLogTypes = new HashMap<>();
+    matchedLogTypes.put(containerId1.toString(), mockContainer1);
+    matchedLogTypes.put(containerId2.toString(), mockContainer2);
+    doReturn(matchedLogTypes).when(cli).getMatchedLogTypesForRunningApp(
+        anyListOf(ContainerLogsRequest.class), anyBoolean(),
+        anyBoolean());
 
     cli.setConf(new YarnConfiguration());
     int exitCode = cli.run(new String[] {"-applicationId", appId.toString()});
@@ -825,7 +849,7 @@ public class TestLogsCLI {
     // printContainerLogsFromRunningApplication twice
     verify(cli, times(2)).printContainerLogsFromRunningApplication(
         any(Configuration.class), logsRequestCaptor.capture(),
-        any(LogCLIHelpers.class), anyBoolean());
+        any(LogCLIHelpers.class), anyBoolean(), anyBoolean());
 
     // Verify that the log-type is "ALL"
     List<ContainerLogsRequest> capturedRequests =
@@ -839,9 +863,12 @@ public class TestLogsCLI {
     mockYarnClient = createMockYarnClientWithException(
         YarnApplicationState.RUNNING, ugi.getShortUserName());
     LogsCLI cli2 = spy(new LogsCLIForTest(mockYarnClient));
+    ContainerLogsRequest newOption = mock(ContainerLogsRequest.class);
+    doReturn(newOption).when(cli2).getMatchedOptionForRunningApp(
+        any(ContainerLogsRequest.class), anyBoolean(), anyBoolean());
     doReturn(0).when(cli2).printContainerLogsFromRunningApplication(
         any(Configuration.class), any(ContainerLogsRequest.class),
-        any(LogCLIHelpers.class), anyBoolean());
+        any(LogCLIHelpers.class), anyBoolean(), anyBoolean());
     doReturn("123").when(cli2).getNodeHttpAddressFromRMWebString(
         any(ContainerLogsRequest.class));
     cli2.setConf(new YarnConfiguration());
@@ -851,7 +878,7 @@ public class TestLogsCLI {
     assertTrue(exitCode == 0);
     verify(cli2, times(1)).printContainerLogsFromRunningApplication(
         any(Configuration.class), logsRequestCaptor.capture(),
-        any(LogCLIHelpers.class), anyBoolean());
+        any(LogCLIHelpers.class), anyBoolean(), anyBoolean());
   }
 
   @Test (timeout = 15000)

http://git-wip-us.apache.org/repos/asf/hadoop/blob/ec2ae306/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java
index 97b78ec..887d92d 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java
@@ -350,10 +350,10 @@ public class LogCLIHelpers implements Configurable {
   }
 
   @Private
-  public Set<String> listContainerLogs(ContainerLogsRequest options)
-      throws IOException {
+  public Set<ContainerLogFileInfo> listContainerLogs(
+      ContainerLogsRequest options) throws IOException {
     List<ContainerLogMeta> containersLogMeta;
-    Set<String> logTypes = new HashSet<String>();
+    Set<ContainerLogFileInfo> logTypes = new HashSet<ContainerLogFileInfo>();
     try {
       containersLogMeta = getFileController(options.getAppId(),
           options.getAppOwner()).readAggregatedLogsMeta(
@@ -364,7 +364,7 @@ public class LogCLIHelpers implements Configurable {
     }
     for (ContainerLogMeta logMeta: containersLogMeta) {
       for (ContainerLogFileInfo fileInfo : logMeta.getContainerLogMeta()) {
-        logTypes.add(fileInfo.getFileName());
+        logTypes.add(fileInfo);
       }
     }
     return logTypes;

http://git-wip-us.apache.org/repos/asf/hadoop/blob/ec2ae306/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/tfile/LogAggregationTFileController.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/tfile/LogAggregationTFileController.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/tfile/LogAggregationTFileController.java
index 989b326..92e3a08 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/tfile/LogAggregationTFileController.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/tfile/LogAggregationTFileController.java
@@ -268,6 +268,12 @@ public class LogAggregationTFileController
     }
     while (nodeFiles.hasNext()) {
       FileStatus thisNodeFile = nodeFiles.next();
+      if (thisNodeFile.getPath().getName().equals(appId + ".har")) {
+        Path p = new Path("har:///"
+            + thisNodeFile.getPath().toUri().getRawPath());
+        nodeFiles = HarFs.get(p.toUri(), conf).listStatusIterator(p);
+        continue;
+      }
       if (nodeIdStr != null) {
         if (!thisNodeFile.getPath().getName().contains(nodeIdStr)) {
           continue;


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org