YARN-9602. Use logger format in Container Executor. Contributed by Abhishek Modi.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java

@@ -290,7 +290,7 @@ public abstract class ContainerExecutor implements Configurable {
     Path pidPath = getPidFilePath(containerId);
 
     if (pidPath == null) {
-      LOG.warn(containerId + " is not active, returning terminated error");
+      LOG.warn("{} is not active, returning terminated error", containerId);
 
       return ExitCode.TERMINATED.getExitCode();
     }
@@ -301,7 +301,7 @@ public abstract class ContainerExecutor implements Configurable {
       throw new IOException("Unable to determine pid for " + containerId);
     }
 
-    LOG.info("Reacquiring " + containerId + " with pid " + pid);
+    LOG.info("Reacquiring {} with pid {}", containerId, pid);
 
     ContainerLivenessContext livenessContext = new ContainerLivenessContext
         .Builder()
@@ -322,7 +322,7 @@ public abstract class ContainerExecutor implements Configurable {
 
     while (!file.exists() && msecLeft >= 0) {
       if (!isContainerActive(containerId)) {
-        LOG.info(containerId + " was deactivated");
+        LOG.info("{} was deactivated", containerId);
 
         return ExitCode.TERMINATED.getExitCode();
       }
@@ -754,7 +754,7 @@ public abstract class ContainerExecutor implements Configurable {
       ipAndHost[0] = address.getHostAddress();
       ipAndHost[1] = address.getHostName();
     } catch (UnknownHostException e) {
-      LOG.error("Unable to get Local hostname and ip for " + container
+      LOG.error("Unable to get Local hostname and ip for {}", container
           .getContainerId(), e);
     }
     return ipAndHost;
@@ -782,7 +782,7 @@ public abstract class ContainerExecutor implements Configurable {
    *          the Container
    */
   public void pauseContainer(Container container) {
-    LOG.warn(container.getContainerId() + " doesn't support pausing.");
+    LOG.warn("{} doesn't support pausing.", container.getContainerId());
     throw new UnsupportedOperationException();
   }
 
@@ -793,7 +793,7 @@ public abstract class ContainerExecutor implements Configurable {
    *          the Container
    */
   public void resumeContainer(Container container) {
-    LOG.warn(container.getContainerId() + " doesn't support resume.");
+    LOG.warn("{} doesn't support resume.", container.getContainerId());
     throw new UnsupportedOperationException();
   }
 
@@ -835,7 +835,7 @@ public abstract class ContainerExecutor implements Configurable {
       try {
         pid = ProcessIdFileReader.getProcessId(pidFile);
       } catch (IOException e) {
-        LOG.error("Got exception reading pid from pid-file " + pidFile, e);
+        LOG.error("Got exception reading pid from pid-file {}", pidFile, e);
       }
     }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java

@@ -167,16 +167,15 @@ public class DefaultContainerExecutor extends ContainerExecutor {
     String tokenFn = String.format(TOKEN_FILE_NAME_FMT, locId);
     Path tokenDst = new Path(appStorageDir, tokenFn);
     copyFile(nmPrivateContainerTokensPath, tokenDst, user);
-    LOG.info("Copying from " + nmPrivateContainerTokensPath
-        + " to " + tokenDst);
+    LOG.info("Copying from {} to {}", nmPrivateContainerTokensPath, tokenDst);
 
 
     FileContext localizerFc =
         FileContext.getFileContext(lfs.getDefaultFileSystem(), getConf());
     localizerFc.setUMask(lfs.getUMask());
     localizerFc.setWorkingDirectory(appStorageDir);
-    LOG.info("Localizer CWD set to " + appStorageDir + " = " 
-        + localizerFc.getWorkingDirectory());
+    LOG.info("Localizer CWD set to {} = {}", appStorageDir,
+        localizerFc.getWorkingDirectory());
 
     ContainerLocalizer localizer =
         createContainerLocalizer(user, appId, locId, tokenFn, localDirs,
@@ -292,8 +291,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
     if (pidFile != null) {
       sb.writeLocalWrapperScript(launchDst, pidFile);
     } else {
-      LOG.info("Container " + containerIdStr
-          + " pid file not set. Returning terminated error");
+      LOG.info("Container {} pid file not set. Returning terminated error",
+          containerIdStr);
       return ExitCode.TERMINATED.getExitCode();
     }
     
@@ -312,8 +311,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       if (isContainerActive(containerId)) {
         shExec.execute();
       } else {
-        LOG.info("Container " + containerIdStr +
-            " was marked as inactive. Returning terminated error");
+        LOG.info("Container {} was marked as inactive. "
+            + "Returning terminated error", containerIdStr);
         return ExitCode.TERMINATED.getExitCode();
       }
     } catch (IOException e) {
@@ -321,14 +320,14 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         return -1;
       }
       int exitCode = shExec.getExitCode();
-      LOG.warn("Exit code from container " + containerId + " is : " + exitCode);
+      LOG.warn("Exit code from container {} is : {}", containerId, exitCode);
       // 143 (SIGTERM) and 137 (SIGKILL) exit codes means the container was
       // terminated/killed forcefully. In all other cases, log the
       // container-executor's output
       if (exitCode != ExitCode.FORCE_KILLED.getExitCode()
           && exitCode != ExitCode.TERMINATED.getExitCode()) {
-        LOG.warn("Exception from container-launch with container ID: "
-            + containerId + " and exit code: " + exitCode , e);
+        LOG.warn("Exception from container-launch with container ID: {}"
+            + " and exit code: {}", containerId, exitCode, e);
 
         StringBuilder builder = new StringBuilder();
         builder.append("Exception from container-launch.\n")
@@ -386,13 +385,13 @@ public class DefaultContainerExecutor extends ContainerExecutor {
     String[] command = getRunCommand(wrapperScriptPath,
         containerIdStr, user, pidFile, this.getConf(), resource);
 
-      LOG.info("launchContainer: " + Arrays.toString(command));
-      return new ShellCommandExecutor(
-          command,
-          workDir,
-          environment,
-          0L,
-          false);
+    LOG.info("launchContainer: {}", Arrays.toString(command));
+    return new ShellCommandExecutor(
+        command,
+        workDir,
+        environment,
+        0L,
+        false);
   }
 
   /**
@@ -648,19 +647,19 @@ public class DefaultContainerExecutor extends ContainerExecutor {
     List<Path> baseDirs = ctx.getBasedirs();
 
     if (baseDirs == null || baseDirs.size() == 0) {
-      LOG.info("Deleting absolute path : " + subDir);
+      LOG.info("Deleting absolute path : {}", subDir);
       if (!lfs.delete(subDir, true)) {
         //Maybe retry
-        LOG.warn("delete returned false for path: [" + subDir + "]");
+        LOG.warn("delete returned false for path: [{}]", subDir);
       }
       return;
     }
     for (Path baseDir : baseDirs) {
       Path del = subDir == null ? baseDir : new Path(baseDir, subDir);
-      LOG.info("Deleting path : " + del);
+      LOG.info("Deleting path : {}", del);
       try {
         if (!lfs.delete(del, true)) {
-          LOG.warn("delete returned false for path: [" + del + "]");
+          LOG.warn("delete returned false for path: [{}]", del);
         }
       } catch (FileNotFoundException e) {
         continue;
@@ -743,7 +742,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       try {
         space = getDiskFreeSpace(curBase);
       } catch (IOException e) {
-        LOG.warn("Unable to get Free Space for " + curBase.toString(), e);
+        LOG.warn("Unable to get Free Space for {}", curBase, e);
       }
       availableOnDisk[i++] = space;
       totalAvailable += space;
@@ -823,7 +822,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         createDir(getUserCacheDir(new Path(localDir), user), userperms, true,
             user);
       } catch (IOException e) {
-        LOG.warn("Unable to create the user directory : " + localDir, e);
+        LOG.warn("Unable to create the user directory : {}", localDir, e);
         continue;
       }
       userDirStatus = true;
@@ -850,7 +849,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
    */
   void createUserCacheDirs(List<String> localDirs, String user)
       throws IOException {
-    LOG.info("Initializing user " + user);
+    LOG.info("Initializing user {}", user);
 
     boolean appcacheDirStatus = false;
     boolean distributedCacheDirStatus = false;
@@ -865,7 +864,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         createDir(appDir, appCachePerms, true, user);
         appcacheDirStatus = true;
       } catch (IOException e) {
-        LOG.warn("Unable to create app cache directory : " + appDir, e);
+        LOG.warn("Unable to create app cache directory : {}", appDir, e);
       }
       // create $local.dir/usercache/$user/filecache
       final Path distDir = getFileCacheDir(localDirPath, user);
@@ -873,7 +872,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         createDir(distDir, fileperms, true, user);
         distributedCacheDirStatus = true;
       } catch (IOException e) {
-        LOG.warn("Unable to create file cache directory : " + distDir, e);
+        LOG.warn("Unable to create file cache directory : {}", distDir, e);
       }
     }
     if (!appcacheDirStatus) {
@@ -911,7 +910,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         createDir(fullAppDir, appperms, true, user);
         initAppDirStatus = true;
       } catch (IOException e) {
-        LOG.warn("Unable to create app directory " + fullAppDir.toString(), e);
+        LOG.warn("Unable to create app directory {}",
+            fullAppDir, e);
       }
     }
     if (!initAppDirStatus) {
@@ -942,7 +942,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       try {
         createDir(appLogDir, appLogDirPerms, true, user);
       } catch (IOException e) {
-        LOG.warn("Unable to create the app-log directory : " + appLogDir, e);
+        LOG.warn("Unable to create the app-log directory : {}", appLogDir, e);
         continue;
       }
       appLogDirStatus = true;
@@ -976,8 +976,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       try {
         createDir(containerLogDir, containerLogDirPerms, true, user);
       } catch (IOException e) {
-        LOG.warn("Unable to create the container-log directory : "
-            + appLogDir, e);
+        LOG.warn("Unable to create the container-log directory : {}",
+            appLogDir, e);
         continue;
       }
       containerLogDirStatus = true;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java

@@ -210,8 +210,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
         YarnConfiguration.NM_NONSECURE_MODE_LIMIT_USERS,
         YarnConfiguration.DEFAULT_NM_NONSECURE_MODE_LIMIT_USERS);
     if (!containerLimitUsers) {
-      LOG.warn(YarnConfiguration.NM_NONSECURE_MODE_LIMIT_USERS +
-          ": impersonation without authentication enabled");
+      LOG.warn("{}: impersonation without authentication enabled",
+          YarnConfiguration.NM_NONSECURE_MODE_LIMIT_USERS);
     }
   }
 
@@ -304,8 +304,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
           false);
     } catch (PrivilegedOperationException e) {
       int exitCode = e.getExitCode();
-      LOG.warn("Exit code from container executor initialization is : "
-          + exitCode, e);
+      LOG.warn("Exit code from container executor initialization is : {}",
+          exitCode, e);
 
       throw new IOException("Linux container executor not configured properly"
           + " (error=" + exitCode + ")", e);
@@ -406,8 +406,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
 
     } catch (PrivilegedOperationException e) {
       int exitCode = e.getExitCode();
-      LOG.warn("Exit code from container " + locId + " startLocalizer is : "
-          + exitCode, e);
+      LOG.warn("Exit code from container {} startLocalizer is : {}",
+          locId, exitCode, e);
 
       throw new IOException("Application " + appId + " initialization failed" +
           " (exitCode=" + exitCode + ") with output: " + e.getOutput(), e);
@@ -530,8 +530,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
               numaArgs = op.getArguments();
               break;
             default:
-              LOG.warn("PrivilegedOperation type unsupported in launch: "
-                  + op.getOperationType());
+              LOG.warn("PrivilegedOperation type unsupported in launch: {}",
+                  op.getOperationType());
             }
           }
 
@@ -585,14 +585,14 @@ public class LinuxContainerExecutor extends ContainerExecutor {
   private int handleExitCode(ContainerExecutionException e, Container container,
       ContainerId containerId) throws ConfigurationException {
     int exitCode = e.getExitCode();
-    LOG.warn("Exit code from container " + containerId + " is : " + exitCode);
+    LOG.warn("Exit code from container {} is : {}", containerId, exitCode);
     // 143 (SIGTERM) and 137 (SIGKILL) exit codes means the container was
     // terminated/killed forcefully. In all other cases, log the
     // output
     if (exitCode != ContainerExecutor.ExitCode.FORCE_KILLED.getExitCode()
         && exitCode != ContainerExecutor.ExitCode.TERMINATED.getExitCode()) {
-      LOG.warn("Exception from container-launch with container ID: "
-          + containerId + " and exit code: " + exitCode, e);
+      LOG.warn("Exception from container-launch with container ID: {} "
+          + "and exit code: {}", containerId, exitCode, e);
 
       StringBuilder builder = new StringBuilder();
       builder.append("Exception from container-launch.\n")
@@ -703,7 +703,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
           resourceHandlerChain.reacquireContainer(containerId);
         } catch (ResourceHandlerException e) {
           LOG.warn("ResourceHandlerChain.reacquireContainer failed for " +
-              "containerId: " + containerId + " Exception: " + e);
+              "containerId: {} Exception: ", containerId, e);
         }
       }
 
@@ -741,8 +741,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
           .getValue()) {
         return false;
       }
-      LOG.warn("Error in signalling container " + pid + " with " + signal
-          + "; exit = " + retCode, e);
+      LOG.warn("Error in signalling container {} with {}; exit = {}",
+          pid, signal, retCode, e);
       logOutput(e.getOutput());
       throw new IOException("Problem signalling container " + pid + " with "
           + signal + "; output: " + e.getOutput() + " and exitCode: "
@@ -775,8 +775,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
       if (retCode != 0) {
         return false;
       }
-      LOG.warn("Error in reaping container "
-          + container.getContainerId().toString() + " exit = " + retCode, e);
+      LOG.warn("Error in reaping container {} exit = {}",
+          container.getContainerId(), retCode, e);
       logOutput(e.getOutput());
       throw new IOException("Error in reaping container "
           + container.getContainerId().toString() + " exit = " + retCode, e);
@@ -804,8 +804,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
       if (retCode != 0) {
         return new IOStreamPair(null, null);
       }
-      LOG.warn("Error in executing container interactive shell"
-          + ctx + " exit = " + retCode, e);
+      LOG.warn("Error in executing container interactive shell {} exit = {}",
+          ctx, retCode, e);
       logOutput(e.getOutput());
       throw new ContainerExecutionException(
           "Error in executing container interactive shel" + ctx.getContainer()
@@ -837,12 +837,12 @@ public class LinuxContainerExecutor extends ContainerExecutor {
 
     List<String> pathsToDelete = new ArrayList<String>();
     if (baseDirs == null || baseDirs.size() == 0) {
-      LOG.info("Deleting absolute path : " + dir);
+      LOG.info("Deleting absolute path : {}", dir);
       pathsToDelete.add(dirString);
     } else {
       for (Path baseDir : baseDirs) {
         Path del = dir == null ? baseDir : new Path(baseDir, dir);
-        LOG.info("Deleting path : " + del);
+        LOG.info("Deleting path : {}", del);
         pathsToDelete.add(del.toString());
         deleteAsUserOp.appendArgs(baseDir.toUri().getPath());
       }
@@ -857,8 +857,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
           false);
     }   catch (PrivilegedOperationException e) {
       int exitCode = e.getExitCode();
-      LOG.error("DeleteAsUser for " + StringUtils.join(" ", pathsToDelete)
-          + " returned with exit code: " + exitCode, e);
+      LOG.error("DeleteAsUser for {} returned with exit code: {}",
+          StringUtils.join(" ", pathsToDelete), exitCode, e);
     }
   }
 
@@ -894,8 +894,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
         }
       }
     } catch (PrivilegedOperationException e) {
-      LOG.error("ListAsUser for " + dir + " returned with exit code: "
-          + e.getExitCode(), e);
+      LOG.error("ListAsUser for {} returned with exit code: {}",
+          dir, e.getExitCode(), e);
     }
 
     return files.toArray(new File[files.size()]);
@@ -971,14 +971,14 @@ public class LinuxContainerExecutor extends ContainerExecutor {
       if (DockerCommandExecutor.isRemovable(
           DockerCommandExecutor.getContainerStatus(containerId, privOpExecutor,
               nmContext))) {
-        LOG.info("Removing Docker container : " + containerId);
+        LOG.info("Removing Docker container : {}", containerId);
         DockerRmCommand dockerRmCommand = new DockerRmCommand(containerId,
             ResourceHandlerModule.getCgroupsRelativeRoot());
         DockerCommandExecutor.executeDockerCommand(dockerRmCommand, containerId,
             null, privOpExecutor, false, nmContext);
       }
     } catch (ContainerExecutionException e) {
-      LOG.warn("Unable to remove docker container: " + containerId);
+      LOG.warn("Unable to remove docker container: {}", containerId);
     }
   }
 
@@ -1005,7 +1005,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
     List<String> localDirs = dirsHandler.getLocalDirs();
     if (file.exists()) {
       if (!file.delete()) {
-        LOG.warn("Unable to delete " + sysFSPath.toString());
+        LOG.warn("Unable to delete {}", sysFSPath);
       }
     }
     if (file.createNewFile()) {