Sangjin Lee commented on YARN-3045:

I took a quick pass at the latest patch. First, could you look at the 
checkstyle issue and the unit test failure?

I think the unit test failure is an "existing" issue, but since you looked at 
it for YARN-3792, it'd be great if you could take another look. It looks like 
even the APPLICATION_CREATED_EVENT might be seeing the race condition?

- I'm not 100% clear about the naming convention, but I was under the 
impression that we're sticking with the name "timelineservice" as the package 
name? Is it not the case?
- l.223: minor nit, but let's make inner classes static unless they need to be 
- l.252: I'm a bit puzzled by the hashCode override; is it necessary? If so, 
then we should also override equals. And also, why is it going by only on the 
app id?
- l.296: the same question here

> [Event producers] Implement NM writing container lifecycle events to ATS
> ------------------------------------------------------------------------
>                 Key: YARN-3045
>                 URL: https://issues.apache.org/jira/browse/YARN-3045
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Sangjin Lee
>            Assignee: Naganarasimha G R
>         Attachments: YARN-3045-YARN-2928.002.patch, 
> YARN-3045-YARN-2928.003.patch, YARN-3045-YARN-2928.004.patch, 
> YARN-3045.20150420-1.patch
> Per design in YARN-2928, implement NM writing container lifecycle events and 
> container system metrics to ATS.

This message was sent by Atlassian JIRA

Reply via email to