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

Sunil G commented on YARN-4143:
-------------------------------

Hi [~adhoot]
Thank you for sharing the patch. 
Yes, this overall helps to avoid extra calls to RMAppAttempt to see AM 
container is launched.

I have couple of minor reasons for the alternative suggestion earlier.
1. It can avoid the schedulers contacting RMApp or RMAppAttempt if possible. 
Because the accessibility to get RMAppAttempt looks slightly trickier. but I am 
agreeing that this correct and used many places. 
{code}
    RMAppAttempt appAttempt =
        rmContext.getRMApps().get(applicationId).getCurrentAppAttempt();
{code}

In my opinion it will be good if RMAppAttempt can update 
{{SchedulerApplicationAttempt}}, it will look more clean. 
2. RMAppAttempt knows exactly when the AM Container is launched, so its good to 
update app at that time.

Yes, I know its not very strong reason, I mentioned it because it may looks 
more clean this way :). Also It adds an additional API to scheduler. We can 
definitely weigh the usefulness of this, and if its fine, then can be done. 
Thoughts?


> Optimize the check for AMContainer allocation needed by blacklisting and 
> ContainerType
> --------------------------------------------------------------------------------------
>
>                 Key: YARN-4143
>                 URL: https://issues.apache.org/jira/browse/YARN-4143
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Anubhav Dhoot
>            Assignee: Anubhav Dhoot
>         Attachments: YARN-4143.001.patch
>
>
> In YARN-2005 there are checks made to determine if the allocation is for an 
> AM container. This happens in every allocate call and should be optimized 
> away since it changes only once per SchedulerApplicationAttempt



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to