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

Junping Du commented on YARN-1506:
----------------------------------

Thanks [~bikassaha] and [~jianhe] for review and comments!
bq. At this point the previously accepted 4GB request cannot be satisfied and 
the application will get stuck. We may need to follow this up in a different 
jira.
Agree. Agree to discuss this in a separate JIRA. Improve unit tests as your 
previous comments. 

bq. What is more important is that having a shared method declares that these 
pieces of code are related to each other in a logical way (if such a relation 
exists). 
Done. Abstract to a common method.

bq. Since it’s changed to asynchronous, we may change the log to not say 
successfully.
Removed.

bq. IMO, since UpdateNodeResourceWhenUnusableTransition and 
UpdateNodeResourceWhenNonRunningTransition are the same except one extra 
logging, we can do the logging for both and just keep one transition?
Agree. Merged.

bq. if possible, nodeResourceUpdate method can be moved into 
AbstractYarnScheduler, a new common base class for sharing common code among 
all the schedulers.
That's good point. Move to AbstractYarnScheduler now.

bq. SchedulerNode.setTotalResource -> 
SchedulerNode.updateTotalAndAvailableResource() ?
My current thinking is better to keep it as setTotalResource, as the later one 
may mistake people that we can set availableResource as a separate value in 
this method. Actually, we only set totalResource and availableResource is just 
get refresh to keep consistent with totalResource change. May be we can keep it 
here?

bq. UpdateNodeResourceResponse should be an abstract class which implements 
newInstance() method.
Fixed.

bq. AdminService.updateNodeResource should RMAuditLogger to log the operations 
as well.
Fixed.

> 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-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
(v6.2#6252)

Reply via email to