hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sidharta Seethana (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3542) Re-factor support for CPU as a resource using the new ResourceHandler mechanism
Date Sun, 20 Dec 2015 22:38:46 GMT

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

Sidharta Seethana commented on YARN-3542:

Hi Vinod, 

A few comments inline regarding your review comments.


When user sets the old CgroupsLCEResourcesHandler, you are internally resetting it to DefaultLCEResourcesHandler(inside
LinuxContainerExecutor) and using that as a control to stop using the older handler. This
effectively means the old code is not used anymore, and that the new code is stable. This
doesn't map well with the earlier decision to not document the new handlers yet.

Thats a good point. However, the handlers themselves aren’t the issue IMO, but rather the
configuration which might get out of hand as we add more resource handlers. This is especially
true in light of resource profiles being worked in in YARN-3926 - which might require some
changes to how the resource handlers are configured.  However, there should be no issue hooking
into the new handler using the old configuration mechanism. 

Lot more dedup is possible between the new hierarchy and the older hierarchy

I don’t think we should dedup this code. There is no reason to hook into the new code/handlers
for either of the two scenarios being discussed here : 1) deprecate the old handler in which
case there shouldn’t be any reason to make make major changes to it. 2) not deprecate the
old handler because the new handler might not be stable - in which case it doesn’t make
sense to hook into the new handler/code yet. 

Further, specific constants like CPU_PERIOD_US perhaps belong better to the specific implementations
likeCGroupsCpuResourceHandlerImpl instead of the root CpuResourceHandler.
I am assuming you meant this :  ‘the root CGroupsHandler’ not ‘the root CpuResourceHandler’.
 Arguments can be made both ways here : CGroupsHandler already has enums and constants that
are used across multiple resource handler implementations. Some of these cannot be moved out
(e.g the enum, tasks etc) . Moving out some of these to individual handlers is of limited
use and makes it hard to get an quick overview of all the cgroups subsystems/parameters in
use among the various resource handlers. It also creates problems if new handlers for the
same subsystem are created which require using the same cgroup parameters. Cleaning this up
fully would require more extensive refactoring - individual classes for various cgroups subsystems
etc., This is not necessary, IMO. 

We should also deprecate the old LCEResourcesHandler interface, DefaultLCEResourcesHandler
etc. That said, we shouldn't deprecate them or CgroupsLCEResourcesHandler before we make the
newer mechanism public and usable. So may be we should fork off all this deprecation to another
JIRA and only get to it after we publicly document the new mechanism for stable usage.

Yeah, thats a good point again. The new handler itself isn’t the issue - the new configuration
could be. Like I said before, though, I think there should be no problem using the new handler
if the old configuration mechanism is in use. 

> Re-factor support for CPU as a resource using the new ResourceHandler mechanism
> -------------------------------------------------------------------------------
>                 Key: YARN-3542
>                 URL: https://issues.apache.org/jira/browse/YARN-3542
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager
>            Reporter: Sidharta Seethana
>            Assignee: Varun Vasudev
>            Priority: Critical
>         Attachments: YARN-3542.001.patch, YARN-3542.002.patch, YARN-3542.003.patch, YARN-3542.004.patch,
YARN-3542.005.patch, YARN-3542.006.patch
> In YARN-3443 , a new ResourceHandler mechanism was added which enabled easier addition
of new resource types in the nodemanager (this was used for network as a resource - See YARN-2140
). We should refactor the existing CPU implementation ( LinuxContainerExecutor/CgroupsLCEResourcesHandler
) using the new ResourceHandler mechanism. 

This message was sent by Atlassian JIRA

View raw message