[ https://issues.apache.org/jira/browse/MAPREDUCE-6424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15262537#comment-15262537 ]
Sangjin Lee commented on MAPREDUCE-6424: ---------------------------------------- Thanks [~Naganarasimha] for the updated patch! It looks good for the most part. Some feedback: (JobHistoryEventHandler.java) - l.1055-1067: it might be better if we do this only if the metrics is not null (i.e. moving the {{if (timelineMetrics != null)}}) check to the outside - l.1057, 1061: I don't think this event is needed at the application entity (all we need are the metrics) - l.1231: I think this is a debatable point, but since these are final metric values would it be better to write them via {{putEntities()}}? (JobHistoryEventUtils.java) - l.68: This might be a small point, but how about adding {{long timestamp}} as an input argument so that all metrics can have the same timestamp? That way we can precisely set the timestamp if needed. (TestMRTimelineEventHandling.java) - can we add a test to check the metrics for the application entity? It looks like we have a couple of pre-existing findbugs warnings. Did you have a chance to see if they are straightforward to fix? Also, the checkstyle violations with the patch should be easy to fix. Are the unit test failures unrelated? > Store MR counters as timeline metrics instead of event > ------------------------------------------------------ > > Key: MAPREDUCE-6424 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6424 > Project: Hadoop Map/Reduce > Issue Type: Sub-task > Reporter: Junping Du > Assignee: Naganarasimha G R > Labels: yarn-2928-1st-milestone > Attachments: MAPREDUCE-6424-YARN-2928.v1.001.patch, > MAPREDUCE-6424-YARN-2928.v1.002.patch > > > In MAPREDUCE-6327, we make map/reduce counters get encoded from > JobFinishedEvent as timeline events with counters details in JSON format. > We need to store framework specific counters as metrics in timeline service > to support query, aggregation, etc. -- This message was sent by Atlassian JIRA (v6.3.4#6332)