hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kumar Vavilapalli (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-1856) cgroups based memory monitoring for containers
Date Tue, 08 Dec 2015 20:14:11 GMT

    [ 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:

 - 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.
Similarly the default constant.
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

View raw message