PrabhuJoseph commented on code in PR #4742: URL: https://github.com/apache/hadoop/pull/4742#discussion_r948684444
########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java: ########## @@ -364,33 +376,68 @@ public int relaunchContainer(ContainerStartContext ctx) * Create a new {@link ShellCommandExecutor} using the parameters. * * @param wrapperScriptPath the path to the script to execute - * @param containerIdStr the container ID + * @param container the container reference * @param user the application owner's username * @param pidFile the path to the container's PID file - * @param resource this parameter controls memory and CPU limits. * @param workDir If not-null, specifies the directory which should be set * as the current working directory for the command. If null, * the current working directory is not modified. - * @param environment the container environment * @return the new {@link ShellCommandExecutor} * @see ShellCommandExecutor */ protected CommandExecutor buildCommandExecutor(String wrapperScriptPath, - String containerIdStr, String user, Path pidFile, Resource resource, - File workDir, Map<String, String> environment) { - + Container container, String user, Path pidFile, File workDir) { + + String containerIdStr = container.getContainerId().toString(); + Resource resource = container.getResource(); String[] command = getRunCommand(wrapperScriptPath, containerIdStr, user, pidFile, this.getConf(), resource); + if(numaAwarenessEnabled(this.getConf())) { + try { + numaResourceAllocator.init(this.getConf()); + command = addNumaCommands(command, container); + } catch (YarnException e) { + LOG.warn("Exception :- ", e); + LOG.warn("Initializing container without NUMA."); + } + } LOG.info("launchContainer: {}", Arrays.toString(command)); return new ShellCommandExecutor( command, workDir, - environment, + container.getLaunchContext().getEnvironment(), 0L, false); } + String[] addNumaCommands(String[] commands, Container container){ + try { + NumaResourceAllocation numaAllocation = numaResourceAllocator.allocateNumaNodes(container); Review Comment: The allocateNumaNodes call look better if called from launchContainer instead of buildCommandExecutor. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org