[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16907364#comment-16907364
]
Hadoop QA commented on YARN-9098:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
0s{color} | {color:blue} Docker mode activated. {color} |
| {color:red}-1{color} | {color:red} patch {color} | {color:red} 0m 6s{color}
| {color:red} YARN-9098 does not apply to trunk. Rebase required? Wrong Branch?
See https://wiki.apache.org/hadoop/HowToContribute for help. {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12977615/YARN-9098.008.patch |
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/24565/console |
| Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
> 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: Gergely Pollak
>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, YARN-9098.008.patch, YARN-9098.008.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>, 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>) 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.14#76016)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16810802#comment-16810802
]
Hadoop QA commented on YARN-9098:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
16s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 29s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 1m
0s{color} | {color:red}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
in trunk has 2 extant Findbugs warnings. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
21s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
38s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 57s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 27m 17s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 1m
23s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 83m 31s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.yarn.server.nodemanager.TestNodeManagerResync |
| |
hadoop.yarn.server.nodemanager.containermanager.scheduler.TestContainerSchedulerQueuing
|
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12964987/YARN-9098.008.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux a818f758121f 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 77fe51e |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| findbugs |
https://builds.apache.org/job/PreCommit-YARN-Build/23888/artifact/out/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
|
| unit |
https://builds.apache.org/job/PreCommit-YARN-Build/23888/artifact/out/patc
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16777167#comment-16777167 ] Szilard Nemeth commented on YARN-9098: -- Hi [~Jim_Brennan]! Thanks for the verification and for the review! > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16768590#comment-16768590 ] Peter Bacsko commented on YARN-9098: After some discussion with Szilard, now I can +1 this (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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16768515#comment-16768515
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
16s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m
21s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 50s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 3s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
3s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
20s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
20s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
24s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 69m 47s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12958739/YARN-9098.007.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 30b0d747527d 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 7a57974 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23408/testReport/ |
| Max. process+thread count | 445 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
|
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/23408/console |
| Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
> Separate mtab file reader code and cgroups fi
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16768462#comment-16768462 ] Szilard Nemeth commented on YARN-9098: -- 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16767298#comment-16767298
]
Peter Bacsko commented on YARN-9098:
Maybe it's just nitpicking, but...
{noformat}
public List getPathsForController(String controller) {
return mappings.entrySet().stream()
.filter(e -> e.getValue().contains(controller))
.map(Map.Entry::getKey)
.collect(Collectors.toList());
}
{noformat}
Is it ok to use {{contains()}} here? If cpu and cpuacct are mounted to two
different directories, then we might return wrong path for cpu, no? Usually
they're mounted to the same directory like {{/sys/fs/cgroup/cpu,cpuacct}} but
it's something to think about.
> 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
>
>
> 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16763878#comment-16763878
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
26s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 28s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 43s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
3s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
24s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
35s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
46s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 73m 51s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12958083/YARN-9098.006.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux d3e744073065 3.13.0-153-generic #203-Ubuntu SMP Thu Jun 14
08:52:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / fb8c997 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23346/testReport/ |
| Max. process+thread count | 307 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
|
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/23346/console |
| Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
> Separate mtab file reader code and cgroups f
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16763850#comment-16763850 ] Szilard Nemeth commented on YARN-9098: -- Hi [~Jim_Brennan]! Of course, I fixed [~pbacsko] now with the latest patch (006). Please review once again, thanks! > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16763695#comment-16763695 ] Jim Brennan commented on YARN-9098: --- [~snemeth] thanks for updating the patch! I agree with your fix for the bug I spotted. I reran the tests and encountered no failures. Were you going to put up another patch to address [~pbacsko]'s comments and checkstyle issues? > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16762585#comment-16762585
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
22s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
26s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 19s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 23s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 5 new + 11 unchanged - 0 fixed = 16 total (was 11) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 33s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 20m
40s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
28s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 73m 35s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12957875/YARN-9098.005.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 437cb7c69811 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / de804e5 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/23327/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/23327/testReport/ |
| Max. process+thread count | 306 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nod
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16762542#comment-16762542 ] Szilard Nemeth commented on YARN-9098: -- Hi [~Jim_Brennan]! Thanks for pointing out this tricky "bug"! However, I think my code works exactly as before the change. 1. https://github.com/apache/hadoop/blob/194f0b49fb8ae3b876dde82f04f906f0ab0f5bdf/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsHandlerImpl.java#L267 The javadoc even says: "@return the first mount path that has the requested subsystem" 2. https://github.com/apache/hadoop/blob/194f0b49fb8ae3b876dde82f04f906f0ab0f5bdf/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/util/CgroupsLCEResourcesHandler.java#L445 This also returns one path per controller (the first found). The old code behaved the same as my patch, as it only selected the first path found for each controller, even if it was a not existing path. Anyways, as the code is prepared to handle multiple paths for the same controller (this is very unlikely to happen in reality, but I don't want to alter the behaviour), I adjusted the CGroupsControllerPaths#findControllerPathInMountConfig so that it returns the first path that is readable. So for example, if we have one unreadable (not existing) and one readable (existing) path for one controller (e.g. cpu) then this method will always return the readable path instead of returning null sometimes. I carefully checked the test failures you referred to and I think this change will fix all of them. I also tried to run all the testcases 50 times in Intellij, so I'm pretty confident that the change actually fixes the test failures :) What do you think of the updated patch? > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16751123#comment-16751123
]
Peter Bacsko commented on YARN-9098:
Minor things:
1. Delete the {{tmpDir}} in {{testSelectCgroup}} after the test finished
2. Same applies to {{testMtabParsing}}
3. Use assert methods consistently, eg. either static import them or
{{Assert.assert...()}} (this was probably not introduced by your change, but
it's still a nice thing to do)
4. Make {{controllerPaths}} and {{cGroupsMountConfig}} final in
{{CGroupsHandlerImpl}} (if possible)
> 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
>
>
> 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16725212#comment-16725212
]
Jim Brennan commented on YARN-9098:
---
[~snemeth] I found the bug (or at least one bug).
In findControllerPathInMountConfig(), you are not looping:
{noformat}
public static String findControllerPathInMountConfig(String controller,
CGroupsMountConfig mountConfig) {
String path = mountConfig.getPathForController(controller);
if (path != null) {
if (new File(path).canRead()) {
return path;
} else {
LOG.warn(String.format(
"Skipping inaccessible cgroup mount point %s", path));
}
}
return null;
}
{noformat}
If the bad entry for the CPU controller comes before the good entry, then you
will "skip" and return null for the CPU controller. This code path should loop
through all of the mountconfig entries so that it will properly skip bad
entries and find good ones. It also makes me concerned about other uses of
mountConfig.getPathForController() - the current code essentially assumes that
there is only one entry for each controller.
The reason I think I am hitting it and you (and precommit) are not is because
this is a hash map, so the ordering is essentially random depending on the file
paths. Since our file paths are different, we get different orderings, and
since in my case the bad entry comes first, the tests fail for me.
I found this while debugging the testMtabParsing() test.
> 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
>
>
> 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16725136#comment-16725136 ] Jim Brennan commented on YARN-9098: --- [~snemeth] thanks for updating the patch. I will download and retest. > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16724967#comment-16724967
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 27s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
53s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
56s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
56s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 19s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 4 new + 11 unchanged - 0 fixed = 15 total (was 11) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 39s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 19m
11s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
27s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 70m 0s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12952342/YARN-9098.004.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux d61f07cd88cc 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / cf57113 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_191 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/22920/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/22920/testReport/ |
| Max. process+thread count | 424 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16724900#comment-16724900
]
Szilard Nemeth commented on YARN-9098:
--
Hi [~Jim_Brennan]!
Thanks for looking at my patch and taking steps on testing!
First of all: the static import is fixed with the new patch.
Actually, I understand your fear regarding this refactoring but this eliminates
some code duplication and improves the quality of the code significantly.
About the test errors:
These are very strange errors. At this level, they seem to be all test
framework issues, as somehow on your computer, the temp directory storing
cgroups and the cpu controller underneath have not created, which is very
strange as the tests are having assertions on the file and directory creations.
I re-built and ran the testcases with these commands again on my computer,
locally:
1.
{code:java}
mvn clean package -Pdist -DskipTests -Dmaven.javadoc.skip=true{code}
2.
{code:java}
mvn test -pl org.apache.hadoop:hadoop-yarn-server-nodemanager -fae | tee
~/maventest`date +%Y%m%d`
{code}
I don't have test failures on any of the classes where you had, this is the
excerpt of the output:
{noformat}
[INFO] Running
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.138 s
- in
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser
...
[INFO] Running
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsHandlerImpl
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.538 s
- in
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsHandlerImpl
...
[INFO] Running
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsControllerPaths
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.127 s
- in
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsControllerPaths
{noformat}
This is the file listing of my target directory:
{noformat}
??-( szilardnemeth@snemeth-MBP[10:17:19] <0> @YARN-9098 )--(
~/development/apache/hadoop )--
└-$ cat
/Users/szilardnemeth/development/apache/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/target/test-dir/cgroups/025802ba-4abe-4862-942b-beef2d279ca7
none
/Users/szilardnemeth/development/apache/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/target/test-dir/cgroups/cp
cgroup rw,relatime,cpu 0 0
none
/Users/szilardnemeth/development/apache/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/target/test-dir/cgroups/cpu
cgroup rw,relatime,cpu 0 0
none
/Users/szilardnemeth/development/apache/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/target/test-dir/cgroups/blkio
cgroup rw,relatime,blkio 0 0{noformat}
I even tried to remove the test directory (with {{rm -rf
/Users/szilardnemeth/development/apache/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/target/test-dir}})
and re-execute the tests but they work fine.
Anyway, I added some code that better describes the assertion errors plus added
some logging of cgroup paths to 2 testcases.
Could you please rerun the tests with the new code and do a file listing like I
did?
This must be some platform issue I suppose.
The test failures for {{TestCGroupsHandlerImpl}}:
{{TestCGroupsHandlerImpl#createPremountedCgroups}} calls:
{noformat}
File cpuCgroup = new File(parentDir, "cpu");
//and later on...
assertTrue("Directory should be created", cpuCgroup.mkdirs());
{noformat}
This should create cgroups for cpu, and as you can see, it is even asserted
properly.
Thanks!
> 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
>
>
> 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.
>
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16724524#comment-16724524
]
Jim Brennan commented on YARN-9098:
---
[~snemeth] thanks for the patch!
In general the code looks good to me and I like the refactoring, but I don't
know if it is worth the risk to include it (I'll let others chime in on that).
One issue with the code: I don't think this static import is needed in
CgroupsLCEResourcesHandler ?
{noformat}
import static
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.CGroupsControllerPaths.findControllerPathInMountConfig;
{noformat}
More importantly, when I applied this patch to trunk and ran nodemanager unit
tests on my linux system, I had a few failures that I did not see with trunk:
{noformat}
[INFO] Running
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.242 s
<<< FAILURE! - in
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser
[ERROR]
testMtabParsing(org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser)
Time elapsed: 0.166 s <<< FAILURE!
java.lang.AssertionError: expected:<2> but was:<1>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestMtabFileParser.testMtabParsing(TestMtabFileParser.java:60)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at
org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
at
org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
at
org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
at
org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
at
org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
at
org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
at
org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
at
org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
[INFO] Running
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsHandlerImpl
[ERROR] Tests run: 8, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 0.687 s
<<< FAILURE! - in
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsHandlerImpl
[ERROR]
testPreMountedControllerRoot(org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.TestCGroupsHandlerImpl)
Time elapsed: 0.077 s <<< ERROR!
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.ResourceHandlerException:
Controller cpu not mounted. You either need to mount it with
yarn.nodemanager.linux-container-executor.cgroups.mount or mount cgroups before
launching Yarn
at
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.CGroupsHandlerImpl.initializePreMountedCGroupController(CGroupsHandlerImpl.java:287)
at
org.apache.hadoop.yarn.server.nodem
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16723881#comment-16723881 ] Szilard Nemeth commented on YARN-9098: -- [~devaraj.k], [~tangzhankun], [~zyluo], [~sunilg], [~leftnoteasy]: As you have a lot expertise in this area, could you please help me to review (and hopefully commit) this? Thanks! > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16722980#comment-16722980
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
24s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 24m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
20s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
31s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
49s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 7s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
3s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
13s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 25s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 4 new + 11 unchanged - 0 fixed = 15 total (was 11) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
43s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 17s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 20m
29s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
35s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 85m 41s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12952030/YARN-9098.003.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 9939b3c5a903 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 346c0c8 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/22894/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/22894/testReport/ |
| Max. process+thread count | 308 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nod
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[ https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16722927#comment-16722927 ] Szilard Nemeth commented on YARN-9098: -- Patch003 is fixing the checkstyle issues, except the ones that are complaining about missing javadoc on test classes > 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 > > > 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>, 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>) 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
[jira] [Commented] (YARN-9098) Separate mtab file reader code and cgroups file system hierarchy parser code from CGroupsHandlerImpl and ResourceHandlerModule
[
https://issues.apache.org/jira/browse/YARN-9098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16713790#comment-16713790
]
Hadoop QA commented on YARN-9098:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
19s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 20m
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
1s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 20s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 23s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 14 new + 13 unchanged - 0 fixed = 27 total (was 13)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 34s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 19m
8s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
24s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 74m 54s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9098 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12951100/YARN-9098.002.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux dd481ce50bc9 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 1afba83 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/22815/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/22815/testReport/ |
| Max. process+thread count | 338 (vs. ulimit of 1) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-no
