[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621010#comment-14621010 ]
Sangjin Lee commented on YARN-3836: ----------------------------------- Thanks for updating the patch [~gtCarrera9]. I went over the latest patch (v.2), and here is my input: (TimelineEntity.java) - l.109: Nit: actually {{obj instanceof Identifier}} returns false if {{obj}} is {{null}}. Therefore, you can safely omit the {{obj == null}} check. The same goes for the other classes. - l.533: Shouldn't we check for null from {{getIdentifier()}}? We cannot guarantee that it will be called only by callers who checked {{isValid()}} - l.545: same here - l.550: It sounds like now the type takes precedence over the created time in the sort order in this version. Is this intended? If not (timestamp is supposed to be first), it might be a good idea to have {{Identifier}} implement {{Comparable}} as well and use that in {{TimelineEntity.compareTo()}}. (TimelineMetric.java) - l.149-155: it would perform a little faster to check the id first and then the type > add equals and hashCode to TimelineEntity and other classes in the data model > ----------------------------------------------------------------------------- > > Key: YARN-3836 > URL: https://issues.apache.org/jira/browse/YARN-3836 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver > Affects Versions: YARN-2928 > Reporter: Sangjin Lee > Assignee: Li Lu > Attachments: YARN-3836-YARN-2928.001.patch, > YARN-3836-YARN-2928.002.patch > > > Classes in the data model API (e.g. {{TimelineEntity}}, > {{TimelineEntity.Identifer}}, etc.) do not override {{equals()}} or > {{hashCode()}}. This can cause problems when these objects are used in a > collection such as a {{HashSet}}. We should implement these methods wherever > appropriate. -- This message was sent by Atlassian JIRA (v6.3.4#6332)