hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Miklos Szegedi (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5301) NM mount cpu cgroups failed on some systems
Date Fri, 31 Mar 2017 22:03:41 GMT

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

Miklos Szegedi commented on YARN-5301:
--------------------------------------

Thank you, [~templedf] for the review. Most of the comments made sense and I addressed them,
I have a few comments below.
bq. In parseMtab(), should cgroupList be a Set instead?
I checked and we only iterate through these items later, when we add them to the mount point.
My understanding is that Set helps, when we search for the items, so they need to be indexed
at the expense of some memory. That is not the case here. On the other hand, I can change
it if you insist. I kept it for now.
I updated the occurrences I saw to String.format
bq.     Collections.addAll(validCgroups, CGroupController.values());
bq.    HashSet<String> validCgroups = new HashSet<>(Arrays.asList(CGroupController.values()));
Unfortunately, none of these options build to me. addAll requires T... CGroupController.values()
is CGroupController[], not compatible with String[].
bq. cgroupList.retain(validCgroups);
It throws:
{code}
java.lang.UnsupportedOperationException
	at java.util.AbstractList.remove(AbstractList.java:161)
	at java.util.AbstractList$Itr.remove(AbstractList.java:374)
	at java.util.AbstractCollection.retainAll(AbstractCollection.java:410)
	at org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources.CGroupsHandlerImpl.parseMtab(CGroupsHandlerImpl.java:229)
{code}
bq. With all the path element concatenation, have you considered using StringUtils.join()
or a Guava Joiner?
The nice thing I found about the File class is that it is reusable, and adds the separator
automatically, I do not need to check for it or add it explicitely.
bq. In updateCGroupParam(), I don't see the point of the exceptionToThrow. Aren't you getting
the same behavior you would if you just threw the exceptions directly (in the reverse order)?
And more actionable error messages would be nice.
It is not a good practice, even causes a compiler warning to throw in finally. This is the
reason for the change.

> NM mount cpu cgroups failed on some systems
> -------------------------------------------
>
>                 Key: YARN-5301
>                 URL: https://issues.apache.org/jira/browse/YARN-5301
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: sandflee
>            Assignee: Miklos Szegedi
>         Attachments: YARN-5301.000.patch, YARN-5301.001.patch
>
>
> on ubuntu  with linux kernel 3.19, , NM start failed if enable auto mount cgroup. try
command:
> ./bin/container-executor --mount-cgroups yarn-hadoop cpu=/cgroup/cpu    fail
> ./bin/container-executor --mount-cgroups yarn-hadoop cpu,cpuacct=/cgroup/cpu    succ



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

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


Mime
View raw message