[
https://issues.apache.org/jira/browse/YARN-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14290444#comment-14290444
]
zhihai xu commented on YARN-3079:
---------------------------------
thanks [~leftnoteasy] and [~adhoot]'s review,
addressed the comment from [~adhoot] in the new patch YARN-3079.002.patch.
about the comment from [~leftnoteasy],
bq. 1) Suggest to change signature of updateMaximumAllocation(SchedulerNode,
bool) to updateMaximumAllocation(Resource nodeResource, bool), since we only
uses nodeResource here.
This is discussable. I prefer to keep the current signature because the current
signature is more flexible and more meaningful for the other parameter(added
node or removed node). Two nodes can have same nodeResource and you can access
more information from SchedulerNode.
bq. 2) Change resource for a NM is equivalent to
{{updateMaximumAllocation(oldNodeResource, false)}} and
{{updateMaximumAllocation(newNoderesource, true)}}. We can avoid some
duplicated logic.
I think it is not completely equivalent . because when you call
{{updateMaximumAllocation(oldNodeResource, false)}}, you supposed the node is
already removed from the HashMap nodes based on both the implementation of
updateMaximumAllocation and caller of updateMaximumAllocation. But in the
context updateNodeResource, the node whose resource to be changed is still in
the HashMap nodes.
bq. 3) Suggest rename updateMaximumAllocation(void) to
refreshMaximumAllocation() or other name reflects the behavior: "scan all
cluster nodes and get maximum allocation".
good suggestion. refreshMaximumAllocation is very good name. addressed this
comment in the new patch YARN-3079.002.patch.
> Scheduler should also update maximumAllocation when updateNodeResource.
> -----------------------------------------------------------------------
>
> Key: YARN-3079
> URL: https://issues.apache.org/jira/browse/YARN-3079
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: zhihai xu
> Assignee: zhihai xu
> Attachments: YARN-3079.000.patch, YARN-3079.001.patch,
> YARN-3079.002.patch
>
>
> Scheduler should also update maximumAllocation when updateNodeResource.
> Otherwise even the node resource is changed by
> AdminService#updateNodeResource, maximumAllocation won't be changed.
> Also RMNodeReconnectEvent called from
> ResourceTrackerService#registerNodeManager will also trigger
> AbstractYarnScheduler#updateNodeResource being called.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
