Junping Du commented on YARN-1506:

Thanks [~jianhe] for the comments!
bq. We need to check the current state also ? if running send this event, else 
Good point. However, the Node's running status is already checked in 
StateMachine when calling this event hook. The behaviors before sending 
NodeResourceUpdateSchedulerEvent won't change status of RMNode, so we can omit 
the check here?

bq. AdminService is server side implementation, right?
Right. Updated with check resource.

bq. we need to wait for some time after “alloc1Response = am1.schedule();”, as 
the allocations happens asynchronously. 
Nice catch. Updated.

bq. The testResourceOverCommit seems almost duplicated. maybe lets start from 
here, as I think duplicating them now will become review/maintenance overhead 
later. TestWorkPreservingRMRestart has the structure for parameterizing 
scheduler class.
There are also other methods, like: testBlackListNodes(), etc. that we can 
merge into a common piece. I agree that this is a good direction to move on, 
but test code style (not test case coverage ) should be the second citizenship 
especially given this is an important fix and get blocked for long time with so 
many versions. Let's file a separated JIRA to fix it.

bq. how about just merging updateResourceOnSchedulerNode into 
updateNodeResource, as they semantically look the same.
Nice catch. will fix it.

> Replace set resource change on RMNode/SchedulerNode directly with event 
> notification.
> -------------------------------------------------------------------------------------
>                 Key: YARN-1506
>                 URL: https://issues.apache.org/jira/browse/YARN-1506
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, scheduler
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: YARN-1506-v1.patch, YARN-1506-v10.patch, 
> YARN-1506-v11.patch, YARN-1506-v12.patch, YARN-1506-v13.patch, 
> YARN-1506-v14.patch, YARN-1506-v2.patch, YARN-1506-v3.patch, 
> YARN-1506-v4.patch, YARN-1506-v5.patch, YARN-1506-v6.patch, 
> YARN-1506-v7.patch, YARN-1506-v8.patch, YARN-1506-v9.patch
> According to Vinod's comments on YARN-312 
> (https://issues.apache.org/jira/browse/YARN-312?focusedCommentId=13846087&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13846087),
>  we should replace RMNode.setResourceOption() with some resource change event.

This message was sent by Atlassian JIRA

Reply via email to