[
https://issues.apache.org/jira/browse/YARN-4166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15981709#comment-15981709
]
Naganarasimha G R commented on YARN-4166:
-----------------------------------------
Thanks for the patch [~fly_in_gis], Overall approach seems to be fine and
almost similar as what i intended to do, assigning this issue to you and will
further help in getting it committed. Yes you were right that the findbugs
issue is not related to the patch and have raised a separate issue(YARN-6515)
for the same.
Few comments on the patch:
# ContainerExecutor ln no 183: {{updateContainerResource}} can be abstract like
others and need not throw ResourceHandlerException.
# ContainerExecutor ln no 183: {{ResourceHandlerException}} requires to be
thrown for any other case ? May be captured in the javadocs on what scenarios
this exception will be thrown..
# ContainerManagerImpl ln no 1238 : We need to invoke
{{ContainerExecutor().updateContainerResource}}, this will ensure that on
recovery proper resource information are assigned to containers else there is
possibility that the resource update fails before cgroups modifications are
done but on restart NM will report higher/lower cpu ...
# CGroupsCpuResourceHandlerImpl ln no 246, invoking
{{cGroupsHandler.deleteCGroup}} in setupLimits makes sense for the creating new
container, but on update if there is excpetion just deleting the cgroup is not
ideal and as well we might require to propagate the update failure to RM or
fail the container, need to see how its handled (may be this error handling if
not done can be done in other).
# ResourceHandler ln no 72: IIUC no need to return List<PrivilegedOperations>
as we are not taking any decision based on it. Here we just send null in most
of the flows. IIUC PrivilegedOperation is used to squash such operations
related optimization of container launch. And currently they support only for
ADD_PID_TO_CGROUP, hence we can have the signature as void. And if required we
can implement the stack properly when required.
# May be once failure to update scenario handling is finalized we might need to
add test case for the same.
# can you please check whether the checkstyle is applicable to the patch ?
> Support changing container cpu resource
> ---------------------------------------
>
> Key: YARN-4166
> URL: https://issues.apache.org/jira/browse/YARN-4166
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: api, nodemanager, resourcemanager
> Affects Versions: 2.8.0, 3.0.0-alpha2
> Reporter: Jian He
> Assignee: Yang Wang
> Attachments: YARN-4166.001.patch, YARN-4166.002.patch,
> YARN-4166-branch2.8-001.patch
>
>
> Memory resizing is now supported, we need to support the same for cpu.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
