hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Junping Du (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3443) Create a 'ResourceHandler' subsystem to ease addition of support for new resource types on the NM
Date Thu, 09 Apr 2015 01:15:13 GMT

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

Junping Du commented on YARN-3443:
----------------------------------

Thanks [~sidharta-s] for delivering the patch and [~vvasudev] for reviewing. 
Just quickly walk though the patch, the patch looks good in overall. Some minor comments below,
but more comments may come later:
{code}
+        if (LOG.isWarnEnabled()) {
+          LOG.warn("Invalid argument: " + arg);
+        }
{code}
We typically log message directly on INFO or above level (like: warn, error), not necessary
for additional check for LOG.isWarnEnabled(). Also there are some other places for checking
LOG.isWarnEnabled() or even LOG.isErrorEnabled() which should be unnecessary.

{code}
+@InterfaceAudience.Private
+@InterfaceStability.Unstable class CGroupsHandlerImpl
+    implements CGroupsHandler {
{code}
We should separate annotation from the line of calss definition. Also, it should be a public
class?

In ResourceHandler.java (and other places, like: ResourceHandlerChain)
{code}
+  List<PrivilegedOperation> teardown() throws ResourceHandlerException;
{code}
teardown => tearDown


> Create a 'ResourceHandler' subsystem to ease addition of support for new resource types
on the NM
> -------------------------------------------------------------------------------------------------
>
>                 Key: YARN-3443
>                 URL: https://issues.apache.org/jira/browse/YARN-3443
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>            Reporter: Sidharta Seethana
>            Assignee: Sidharta Seethana
>         Attachments: YARN-3443.001.patch, YARN-3443.002.patch, YARN-3443.003.patch, YARN-3443.004.patch
>
>
> The current cgroups implementation is closely tied to supporting CPU as a resource .
We need to separate out CGroups support as well a provide a simple ResourceHandler subsystem
that will enable us to add support for new resource types on the NM - e.g Network, Disk etc.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message