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

Szilard Nemeth edited comment on YARN-9098 at 2/14/19 4:12 PM:
---------------------------------------------------------------

Hi [~pbacsko]!
As the mappings contain the path as key and values as controllers, if 
cpu,cpuset is a directory under /sys/fs/cgroup, the mapping will contain the 
following:
1. cpu --> /sys/fs/cgroup/cpu,cpuacct
2. cpuacct --> /sys/fs/cgroup/cpu,cpuacct
So essentially, cpu and cpuacct will point to the same path. The code that you 
pasted just checks if the controller is contained in the value list. For the 
above example, if the method is invoked with either 'cpu' or 'cpuacct', 
contains will return the same path so I think the code is correct.
I added a more elaborated javadoc comment to the file.


was (Author: snemeth):
As the mappings contain the path as key and values as controllers, if 
cpu,cpuset is a directory under /sys/fs/cgroup, the mapping will contain the 
following:
1. cpu --> /sys/fs/cgroup/cpu,cpuacct
2. cpuacct --> /sys/fs/cgroup/cpu,cpuacct
So essentially, cpu and cpuacct will point to the same path. The code that you 
pasted just checks if the controller is contained in the value list. For the 
above example, if the method is invoked with either 'cpu' or 'cpuacct', 
contains will return the same path so I think the code is correct.
I added a more elaborated javadoc comment to the file.

> Separate mtab file reader code and cgroups file system hierarchy parser code 
> from CGroupsHandlerImpl and ResourceHandlerModule
> ------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: YARN-9098
>                 URL: https://issues.apache.org/jira/browse/YARN-9098
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Szilard Nemeth
>            Priority: Major
>         Attachments: YARN-9098.002.patch, YARN-9098.003.patch, 
> YARN-9098.004.patch, YARN-9098.005.patch, YARN-9098.006.patch, 
> YARN-9098.007.patch
>
>
> Separate mtab file reader code and cgroups file system hierarchy parser code 
> from CGroupsHandlerImpl and ResourceHandlerModule
> CGroupsHandlerImpl has a method parseMtab that parses an mtab file and stores 
> cgroups data.
> CGroupsLCEResourcesHandler also has a method with the same name, with 
> identical code.
> The parser code should be extracted from these places and be added in a new 
> class as this is a separate responsibility.
> As the output of the file parser is a Map<String, Set<String>>, it's better 
> to encapsulate it in a domain object, named 'CGroupsMountConfig' for instance.
> ResourceHandlerModule has a method named parseConfiguredCGroupPath, that is 
> responsible for producing the same results (Map<String, Set<String>>) to 
> store cgroups data, it does not operate on mtab file, but looking at the 
> filesystem for cgroup settings. As the output is the same, CGroupsMountConfig 
> should be used here, too.
> Again, this could should not be part of ResourceHandlerModule as it is a 
> different responsibility.
> One more thing which is strongly related to the methods above is 
> CGroupsHandlerImpl.initializeFromMountConfig: This method processes the 
> result of a parsed mtab file or a parsed cgroups filesystem data and stores 
> file system paths for all available controllers. This method invokes 
> findControllerPathInMountConfig, which is a duplicated in CGroupsHandlerImpl 
> and CGroupsLCEResourcesHandler, so it should be moved to a single place. To 
> store filesystem path and controller mappings, a new domain object could be 
> introduced.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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

Reply via email to