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

Sangjin Lee commented on YARN-5699:
-----------------------------------

Thanks [~rohithsharma] for the patch! Sorry it took me a while to get to this.

I generally agree with pulling some info from events to the entity level, 
especially if they can be part of queries. We do want to be careful not to put 
everything at the entity level, however. We probably want to limit it to *only* 
the things we will actually query.

In more detail,
(NMTimelinePublisher.java)
- l.182: we’re already setting the entity created time in l.190; do we need to 
add it to the entity info? I would say, we don't need this
- l.203: do we need to add diagnostics to the entity level? Not sure if it is 
useful. Can we keep it on the event?
- l.307: the same comment as Varun’s; should be the other way around?
- regarding the container state that was removed in YARN-5156; perhaps we 
should add it as entity info here (with COMPLETED)?

(ContainerMetricsConstants.java)
- l.44: see {{NMTimelinePublisher.java}}

(TimelineServiceV2Publisher.java)
- l.158: same as above; is diagnostics needed at the entity level?
- l.190: this should be {{entity.setInfo()}} not {{tEvent.setInfo()}}
- l.289: same as above

It would be great if we can add some unit tests to check these. Thanks!


> Retrospect yarn entity fields which are publishing in events info fields.
> -------------------------------------------------------------------------
>
>                 Key: YARN-5699
>                 URL: https://issues.apache.org/jira/browse/YARN-5699
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Rohith Sharma K S
>            Assignee: Rohith Sharma K S
>         Attachments: 0001-YARN-5699.YARN-5355.patch, 0001-YARN-5699.patch, 
> 0002-YARN-5699.YARN-5355.patch
>
>
> Currently, all the container information are published at 2 places. Some of 
> them are at entity info(top-level) and some are  at event info. 
> For containers, some of the event info should be published at container info 
> level. For example : container exist status, container state, createdTime, 
> finished time. These are general information to container required for 
> container-report. So it is better to publish at top level info field. 



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

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to