[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-25 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r954575871


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java:
##
@@ -1040,4 +1080,92 @@ public void updateYarnSysFS(Context ctx, String user,
   String appId, String spec) throws IOException {
 throw new ServiceStateException("Implementation unavailable");
   }
+
+  @Override
+  public int reacquireContainer(ContainerReacquisitionContext ctx)
+  throws IOException, InterruptedException {
+try {
+  if (numaResourceAllocator != null) {
+numaResourceAllocator.recoverNumaResource(ctx.getContainerId());
+  }
+  return super.reacquireContainer(ctx);
+} finally {
+  postComplete(ctx.getContainerId());
+}
+  }
+
+  /**
+   * clean up and release of resources.
+   *
+   * @param containerId containerId of running container
+   */
+  public void postComplete(final ContainerId containerId) {
+if (numaResourceAllocator != null) {
+  try {
+numaResourceAllocator.releaseNumaResource(containerId);
+  } catch (ResourceHandlerException e) {
+LOG.warn("NumaResource release failed for " +
+"containerId: {}. Exception: ", containerId, e);
+  }
+}
+  }
+
+  /**
+   * @param resourceAllocation NonNull NumaResourceAllocation object reference
+   * @return Array of numa specific commands
+   */
+  String[] getNumaCommands(NumaResourceAllocation resourceAllocation) {
+String[] numaCommand = new String[3];
+numaCommand[0] = 
this.getConf().get(YarnConfiguration.NM_NUMA_AWARENESS_NUMACTL_CMD,
+YarnConfiguration.DEFAULT_NM_NUMA_AWARENESS_NUMACTL_CMD);
+numaCommand[1] = "--interleave=" + String.join(",", 
resourceAllocation.getMemNodes());
+numaCommand[2] = "--cpunodebind=" + String.join(",", 
resourceAllocation.getCpuNodes());
+return numaCommand;
+
+  }
+
+  /**
+   * @param firstStringArray  Array of String
+   * @param secondStringArray Array of String
+   * @return combined array of string where first elements are from 
firstStringArray
+   * and later are the elements from secondStringArray
+   */
+  String[] concatStringCommands(String[] firstStringArray, String[] 
secondStringArray) {

Review Comment:
   The Numa Commands has to be in prefix.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-24 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r954564319


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java:
##
@@ -372,16 +409,19 @@ public int relaunchContainer(ContainerStartContext ctx)
* as the current working directory for the command. If null,
* the current working directory is not modified.
* @param environment the container environment
+   * @param numaCommands list of prefix numa commands
* @return the new {@link ShellCommandExecutor}
* @see ShellCommandExecutor
*/
-  protected CommandExecutor buildCommandExecutor(String wrapperScriptPath, 
-  String containerIdStr, String user, Path pidFile, Resource resource,
-  File workDir, Map environment) {
-
+  protected CommandExecutor buildCommandExecutor(String wrapperScriptPath,
+String containerIdStr, String user, Path pidFile, 
Resource resource,
+File workDir, Map environment, 
String[] numaCommands) {
+
 String[] command = getRunCommand(wrapperScriptPath,
 containerIdStr, user, pidFile, this.getConf(), resource);
 
+command = concatStringCommands(command, numaCommands);

Review Comment:
   Shall we skip calling this when numaCommands is not passed.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-24 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r954562550


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDefaultContainerExecutor.java:
##
@@ -736,4 +755,197 @@ public void testPickDirectory() throws Exception {
 //new FsPermission(ApplicationLocalizer.LOGDIR_PERM), true);
 //  }
 
+  @Before
+  public void setUp() throws IOException, YarnException {
+yarnConfiguration = new YarnConfiguration();
+setNumaConfig();
+Context mockContext = createAndGetMockContext();
+NMStateStoreService nmStateStoreService =
+mock(NMStateStoreService.class);
+when(mockContext.getNMStateStore()).thenReturn(nmStateStoreService);
+numaResourceAllocator = new NumaResourceAllocator(mockContext) {
+  @Override
+  public String executeNGetCmdOutput(Configuration config)
+  throws YarnRuntimeException {
+return getNumaCmdOutput();
+  }
+};
+
+numaResourceAllocator.init(yarnConfiguration);
+FileContext lfs = FileContext.getLocalFSFileContext();
+containerExecutor = new DefaultContainerExecutor(lfs) {
+  @Override
+  public Configuration getConf() {
+return yarnConfiguration;
+  }
+};
+containerExecutor.setNumaResourceAllocator(numaResourceAllocator);
+mockContainer = mock(Container.class);
+  }
+
+  private void setNumaConfig() {
+yarnConfiguration.set(YarnConfiguration.NM_NUMA_AWARENESS_ENABLED, "true");
+yarnConfiguration.set(YarnConfiguration.NM_NUMA_AWARENESS_READ_TOPOLOGY, 
"true");
+yarnConfiguration.set(YarnConfiguration.NM_NUMA_AWARENESS_NUMACTL_CMD, 
"/usr/bin/numactl");
+  }
+
+
+  private String getNumaCmdOutput() {
+// architecture of 8 cpu cores
+// randomly picked size of memory
+return "available: 2 nodes (0-1)\n\t"
++ "node 0 cpus: 0 2 4 6\n\t"
++ "node 0 size: 73717 MB\n\t"
++ "node 0 free: 73717 MB\n\t"
++ "node 1 cpus: 1 3 5 7\n\t"
++ "node 1 size: 73717 MB\n\t"
++ "node 1 free: 73717 MB\n\t"
++ "node distances:\n\t"
++ "node 0 1\n\t"
++ "0: 10 20\n\t"
++ "1: 20 10";
+  }
+
+  private Context createAndGetMockContext() {
+Context mockContext = mock(Context.class);
+@SuppressWarnings("unchecked")
+ConcurrentHashMap mockContainers = mock(
+ConcurrentHashMap.class);
+mockContainer = mock(Container.class);
+when(mockContainer.getResourceMappings())
+.thenReturn(new ResourceMappings());
+when(mockContainers.get(any())).thenReturn(mockContainer);
+when(mockContext.getContainers()).thenReturn(mockContainers);
+when(mockContainer.getResource()).thenReturn(Resource.newInstance(2048, 
2));
+return mockContext;
+  }
+
+  private void testAllocateNumaResource(String containerId, Resource resource,
+String memNodes, String cpuNodes) 
throws Exception {
+when(mockContainer.getContainerId())
+.thenReturn(ContainerId.fromString(containerId));
+when(mockContainer.getResource()).thenReturn(resource);
+NumaResourceAllocation numaResourceAllocation =
+numaResourceAllocator.allocateNumaNodes(mockContainer);
+String[] commands = 
containerExecutor.getNumaCommands(numaResourceAllocation);
+assertEquals(Arrays.asList(commands), Arrays.asList("/usr/bin/numactl",
+"--interleave=" + memNodes, "--cpunodebind=" + cpuNodes));
+  }
+
+  @Test
+  public void testAllocateNumaMemoryResource() throws Exception {
+// keeping cores constant for testing memory resources
+
+// allocates node 0 for memory and cpu
+testAllocateNumaResource("container_1481156246874_0001_01_01",
+Resource.newInstance(2048, 2), "0", "0");
+
+// allocates node 1 for memory and cpu since allocator uses round robin 
assignment
+testAllocateNumaResource("container_1481156246874_0001_01_02",
+Resource.newInstance(6, 2), "1", "1");
+
+// allocates node 0,1 for memory since there is no sufficient memory in 
any one node
+testAllocateNumaResource("container_1481156246874_0001_01_03",
+Resource.newInstance(8, 2), "0,1", "0");
+
+// returns null since there are no sufficient resources available for the 
request
+when(mockContainer.getContainerId()).thenReturn(
+ContainerId.fromString("container_1481156246874_0001_01_04"));
+when(mockContainer.getResource())
+.thenReturn(Resource.newInstance(8, 2));
+Assert.assertNull(numaResourceAllocator.allocateNumaNodes(mockContainer));
+
+// allocates node 1 for memory and cpu
+testAllocateNumaResource("container_1481156246874_0001_01_05",
+Resource.newInstance(1024, 2), "1", "1");
+  }
+
+  @Test
+  public voi

[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-24 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r954556722


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java:
##
@@ -1040,4 +1080,92 @@ public void updateYarnSysFS(Context ctx, String user,
   String appId, String spec) throws IOException {
 throw new ServiceStateException("Implementation unavailable");
   }
+
+  @Override
+  public int reacquireContainer(ContainerReacquisitionContext ctx)
+  throws IOException, InterruptedException {
+try {
+  if (numaResourceAllocator != null) {
+numaResourceAllocator.recoverNumaResource(ctx.getContainerId());
+  }
+  return super.reacquireContainer(ctx);
+} finally {
+  postComplete(ctx.getContainerId());
+}
+  }
+
+  /**
+   * clean up and release of resources.
+   *
+   * @param containerId containerId of running container
+   */
+  public void postComplete(final ContainerId containerId) {
+if (numaResourceAllocator != null) {
+  try {
+numaResourceAllocator.releaseNumaResource(containerId);
+  } catch (ResourceHandlerException e) {
+LOG.warn("NumaResource release failed for " +
+"containerId: {}. Exception: ", containerId, e);
+  }
+}
+  }
+
+  /**
+   * @param resourceAllocation NonNull NumaResourceAllocation object reference
+   * @return Array of numa specific commands
+   */
+  String[] getNumaCommands(NumaResourceAllocation resourceAllocation) {
+String[] numaCommand = new String[3];
+numaCommand[0] = 
this.getConf().get(YarnConfiguration.NM_NUMA_AWARENESS_NUMACTL_CMD,

Review Comment:
   Better to read this config and initialize in the init.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-21 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r951007590


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java:
##
@@ -350,6 +384,14 @@ public int launchContainer(ContainerStartContext ctx)
   return exitCode;
 } finally {
   if (shExec != null) shExec.close();
+  try {

Review Comment:
   Shall we use the same postComplete method here as well.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-17 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r948695039


##
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 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:
   Reacquire Container has to recover the NUMA Resources.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-17 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r948694728


##
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 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:
   Launch Container has to release the NUMA Resources at finally.



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-17 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r94868


##
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 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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-17 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r948677524


##
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 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());

Review Comment:
   The numaResourceAllocator.init needs to be moved into init() method



-- 
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



[GitHub] [hadoop] PrabhuJoseph commented on a diff in pull request #4742: YARN-11196. NUMA support in DCE

2022-08-17 Thread GitBox


PrabhuJoseph commented on code in PR #4742:
URL: https://github.com/apache/hadoop/pull/4742#discussion_r948675282


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java:
##
@@ -98,6 +104,13 @@ public DefaultContainerExecutor() {
 }
   }
 
+  @VisibleForTesting

Review Comment:
   Looks this constructor is only for Testing purpose. Shall we use setMethod 
instead which is followed in other classes.



-- 
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