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

Sangjin Lee commented on YARN-5792:
-----------------------------------

I went over the latest patch, and had some comments.

(ContainerImpl.java)
- l.170: nit: let's make this final

(ApplicationMaster.java)
- l.315: I don't see a reason it needs to be static. Let's make it an instance 
variable. Nit: let's also make it final.

(JobHistoryEventHandler.java)
- l.1073, 1083: Strictly speaking, we don't need the id prefix for the job id, 
because there is only one job entity per YARN application, right? Should we 
skip setting the prefix for job entities?

(TaskImpl.java)
- l.142: The name is bit too verbose. Could there be a better name for this? 
Would {{launchTime}} cause confusion?

In addition, should we revisit and change {{TimelineEntity.compareTo()}} to use 
the entity id prefix? We may also need to review 
{{TimelineEntityReader.readEntities()}} and see if we need to change it.


> adopt the id prefix for YARN, MR, and DS entities
> -------------------------------------------------
>
>                 Key: YARN-5792
>                 URL: https://issues.apache.org/jira/browse/YARN-5792
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-5355
>            Reporter: Sangjin Lee
>            Assignee: Varun Saxena
>         Attachments: YARN-5792-YARN-5355.01.patch, 
> YARN-5792-YARN-5355.02.patch, YARN-5792-YARN-5355.03.patch, 
> YARN-5792-YARN-5355.04.patch
>
>
> We introduced the entity id prefix to support flexible entity sorting 
> (YARN-5715). We should adopt the id prefix for YARN entities, MR entities, 
> and DS entities to take advantage of the id prefix.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to