Wangda Tan commented on YARN-3079:

Hi [~zxu],
Thanks for taking up this task, just reviewed your patch, some comments
1) Suggest to change signature of updateMaximumAllocation(SchedulerNode, bool) 
to updateMaximumAllocation(Resource nodeResource, bool), since we only uses 
nodeResource here.
2) Change resource for a NM is equivalent to 
{{updateMaximumAllocation(oldNodeResource, false)}} and 
{{updateMaximumAllocation(newNoderesource, true)}}. We can avoid some 
duplicated logic.
3) Suggest rename updateMaximumAllocation(void) to refreshMaximumAllocation() 
or other name reflects the behavior: "scan all cluster nodes and get maximum 
4) Not related to this fix -- I found only max allocation is protected by R/W 
lock, it seems not correct to me, I think we should address it in a separated 
JIRA. Will file a ticket later.


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

Reply via email to