[
https://issues.apache.org/jira/browse/YARN-1856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15047400#comment-15047400
]
Vinod Kumar Vavilapalli commented on YARN-1856:
-----------------------------------------------
Quick comments on the patch:
General
- Should add all the configs to yarn-default.xml, saying they are still early
configs?
- Should update the documentation of pmem-check-enabled, vmem-check-enabled
configs in code and yarn-default.xml to denote their relation to
resource.memory.enabled.
- Actually, given existing memory monitoring mechanism,
NM_MEMORY_RESOURCE_ENABLED is in reality is already true when pmem/vmem checks
are enabled. We need to reconcile the old and new configs some how. May be
memory is always enabled, but if vmem/pmem configs are enabled, use old
handler, otherwise use the new one? Thinking out aloud.
- Does the soft and hard limits also some-how logically relate to
pmem-vmem-ratio? If so, we should hint at that in the documentation.
- Swappiness seems like a cluster configuration defaulting to zero. So far,
this has been an implicit contract with our users, good to document this also
in yarn-default.xml
Code comments
- ResourceHandlerModule
-- Formatting of new code is a little off: the declaration of
{{getCgroupsMemoryResourceHandler()}}. There are other occurrences like this in
that class before in this patch, you may want to fix those.
-- BUG! getCgroupsMemoryResourceHandler() incorrectly locks
DiskResourceHandler instead of MemoryResourceHandler.
- CGroupsMemoryResourceHandlerImpl
-- What is this doing? {{ CGroupsHandler.CGroupController MEMORY =
CGroupsHandler.CGroupController.MEMORY; }} Is it forcing a class-load or
something? Not sure if this is needed. If this is needed, you may want to add a
comment here.
- NM_MEMORY_RESOURCE_CGROUPS_SOFT_LIMIT_PERC ->
NM_MEMORY_RESOURCE_CGROUPS_SOFT_LIMIT_PERCENTAGE. Similarly the default
constant.
- CGROUP_PARAM_MEMORY_HARD_LIMIT_BYTES / CGROUP_PARAM_MEMORY_SOFT_LIMIT_BYTES
/ CGROUP_PARAM_MEMORY_SWAPPINESS can all be static and final.
> cgroups based memory monitoring for containers
> ----------------------------------------------
>
> Key: YARN-1856
> URL: https://issues.apache.org/jira/browse/YARN-1856
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager
> Affects Versions: 2.3.0
> Reporter: Karthik Kambatla
> Assignee: Varun Vasudev
> Attachments: YARN-1856.001.patch, YARN-1856.002.patch,
> YARN-1856.003.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
