hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Miklos Szegedi (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5764) NUMA awareness support for launching containers
Date Mon, 14 Aug 2017 17:10:01 GMT

    [ https://issues.apache.org/jira/browse/YARN-5764?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125987#comment-16125987

Miklos Szegedi commented on YARN-5764:

Thank you, [~devaraj.k] for the patch. I have a few quick comments.
First of all a quick question. Is the list of numa nodes, dynamic, can they be enabled/disabled
143          // Remove from NUMA Resources Manager if enabled
144          if (YarnConfiguration.numaAwarenessEnabled(getConfig())) {
145            containerManager.getNumaNodesScheduler()
146                .releaseNumaResources(containerId);
147          }
It is probably a safer practice to release the resources, if containerManager.getNumaNodesScheduler()
is not null
I saw this pattern at the other regexes as well. Should not you use \\s+ instead of \\s* in
some places?
([0-9]*) should be ([0-9]+)
Should not this be ([0-9\\s]+)?
Is there any reason to throw YarnRuntimeException instead of YarnException in serviceInit?
105              long memory = parseMemory(outputLines, nodeIdRange);
Note: Technically this is not a node Id range anymore, if it is not a space. Also, could it
contain commas?
115            long mem = conf.getLong(
116                "yarn.nodemanager.numa-awareness." + nodeId + ".memory", 0);
117            int cpus = conf
118                .getInt("yarn.nodemanager.numa-awareness." + nodeId + ".cpus", 0);
It is probably a good idea to default here to some meaningful value like 1024 for memory and
1 for cpu.
132        String[] args = new String[] {"numactl", "--hardware"};
It might be safer to default this to /usr/bin/numactl and make it configurable.
131      String executeNGetCmdOutput() throws YarnRuntimeException {
YarnRuntimeException is a runtime exception, it does not need to be declared. However, it
is better to use YarnException, I think.
172              } else if (KB.equals(units)) {
173                memory = memory / 1024;
174              }
If memory equals 100 KB, this will become 100/1024=0 as long. Is this expected. We might need
a check that to throw, if memory is 0. Also there is no check for negative values.
52      public synchronized boolean isResourcesAvailable(Resource resource) {
53        LOG.info(
54            "Memory available:" + (totalMemory - usedMemory) + ", CPUs available:"
55                + (totalCpus - usedCpus) + ", requested:" + resource);
This is running in a loop, so it might be better to LOG.debug

> NUMA awareness support for launching containers
> -----------------------------------------------
>                 Key: YARN-5764
>                 URL: https://issues.apache.org/jira/browse/YARN-5764
>             Project: Hadoop YARN
>          Issue Type: New Feature
>          Components: nodemanager, yarn
>            Reporter: Olasoji
>            Assignee: Devaraj K
>         Attachments: NUMA Awareness for YARN Containers.pdf, NUMA Performance Results.pdf,
YARN-5764-v0.patch, YARN-5764-v1.patch, YARN-5764-v2.patch
> The purpose of this feature is to improve Hadoop performance by minimizing costly remote
memory accesses on non SMP systems. Yarn containers, on launch, will be pinned to a specific
NUMA node and all subsequent memory allocations will be served by the same node, reducing
remote memory accesses. The current default behavior is to spread memory across all NUMA nodes.

This message was sent by Atlassian JIRA

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

View raw message