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

Karthik Kambatla commented on YARN-2868:
----------------------------------------

Sorry for coming in late. Few comments on the patch:
# FairScheduler
## I think we should move the logic of computing the delay for the launch of 
first container (i.e., allocation Time - request Time) to a method outside 
allocate. We would want to keep {{allocate()}} as simple and easy to read as 
possible. May be, we could compute that in 
{{SchedulerApplicationAttempt#trySetFirstContainerAllocatedTime}}? 
## Nit: I don't see the need for storing current time in {{start}}. Same goes 
for {{firstContainerAllocatedTime}}
## Comment: I feel we should avoid looking up current time after allocation, 
since we care about it only once for the first container. Again, adding a check 
there would be leaking TMI into {{allocate}}. So, I guess we shouldn't. 
## If possible, we should calculate delay outside of the synchronized block. 

> Add metric for initial container launch time to FairScheduler
> -------------------------------------------------------------
>
>                 Key: YARN-2868
>                 URL: https://issues.apache.org/jira/browse/YARN-2868
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Ray Chiang
>            Assignee: Ray Chiang
>              Labels: metrics, supportability
>         Attachments: YARN-2868-01.patch, YARN-2868.002.patch, 
> YARN-2868.003.patch, YARN-2868.004.patch, YARN-2868.005.patch, 
> YARN-2868.006.patch, YARN-2868.007.patch, YARN-2868.008.patch
>
>
> Add a metric to measure the latency between "starting container allocation" 
> and "first container actually allocated".



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

Reply via email to