[
https://issues.apache.org/jira/browse/YARN-6668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16117264#comment-16117264
]
Haibo Chen commented on YARN-6668:
----------------------------------
Thanks [~miklos.szeg...@cloudera.com] for the update! I have a few follow-up
knits:
1) The change in ResourceCalculatorProcessTree.java is no longer needed.
2) Can we add @Override annotation to CGroupsHandlerImpl.getControllerPath()?
3) The CGroupsResoureCalculator now takes precedence over user-configuration.
4) There's an unused import java.io.FileNotFoundException; in
CGroupsResourceCalculator
5) ContainersMonitorImpl is missing an import of YarnException, which leads to
compilation failure
6) performance is misspelled as "perforance" in
ContainersMonitorImpl.getResourceCalculatorProcessTree()
7) Now that we separate getCgroupHandlers from initCgroupHandlers in
ResourceHandlerModule, and calls getCgroupHandlers() in
CGroupsResourceCalculator to determine whether it is available. I think we have
an implicit assumption here that CGroup is initialized before
CGroupsResourceCalculator.isAvailable() is called. Can we add a comment in
isAvailable to make it more obvious?
8) In CGroupsResourceCalculator constructor, should we initialize jiffyLengthMs
first and then cpuTimeTracker to avoid the cases where cpuTimeTracker get -1 as
the jiffyLength whereas jiffyLengthMs is set to 10?
9) We call setCGroupFilePaths() in constructor which reads file. Can we move
that out into an init() method instead?
10) the lock around firstError is unnecessary if we make firstError volatile.
11) TestCGroupsResourceCalculator.testNoPid() is misnamed in that the
constructor actually takes a pid '1234'. Likewise, testNoCGgroup is testing
calculator.getRssMemorySize(0) returns UNAVAILABLE, right?
12) How about we reference '/tmp/' + this.getClass().getName() as a static
field profcBase? Optionally, we could then move all deletions of procfs into a
teardown method.
13) testNoMemoryCGgroupMount can use a ExpectedException to replace the
AssertTrue in the catch block.
> Use cgroup to get container resource utilization
> ------------------------------------------------
>
> Key: YARN-6668
> URL: https://issues.apache.org/jira/browse/YARN-6668
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager
> Affects Versions: 3.0.0-alpha3
> Reporter: Haibo Chen
> Assignee: Miklos Szegedi
> Attachments: YARN-6668.000.patch, YARN-6668.001.patch,
> YARN-6668.002.patch
>
>
> Container Monitor relies on proc file system to get container resource
> utilization, which is not as efficient as reading cgroup accounting. We
> should in NM, when cgroup is enabled, read cgroup stats instead.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
