[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16777136#comment-16777136
]
Jim Brennan commented on YARN-9098:
-----------------------------------
I downloaded patch 007, built it and ran unit tests.
+1 this looks good to me (non-binding).
> 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: [email protected]
For additional commands, e-mail: [email protected]
