[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15369819#comment-15369819 ] Hudson commented on YARN-3836: -- SUCCESS: Integrated in Hadoop-trunk-Commit #10074 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/10074/]) YARN-3836. add equals and hashCode to TimelineEntity and other classes (sjlee: rev 57d8dc2fb79be050d53d77a0a1def607a5012288) * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/timelineservice/TestTimelineServiceRecords.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEvent.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineMetric.java > 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 > Fix For: YARN-2928 > > Attachments: YARN-3836-YARN-2928.001.patch, > YARN-3836-YARN-2928.002.patch, YARN-3836-YARN-2928.003.patch, > YARN-3836-YARN-2928.004.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) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621559#comment-14621559 ] Sangjin Lee commented on YARN-3836: --- Great. I'll commit the patch this evening unless there are further comments. > 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, YARN-3836-YARN-2928.003.patch, > YARN-3836-YARN-2928.004.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621545#comment-14621545 ] Zhijie Shen commented on YARN-3836: --- +1 LGTM > 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, YARN-3836-YARN-2928.003.patch, > YARN-3836-YARN-2928.004.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621501#comment-14621501 ] Sangjin Lee commented on YARN-3836: --- The latest patch (v.4) looks good to me. Any other comments from anyone? > 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, YARN-3836-YARN-2928.003.patch, > YARN-3836-YARN-2928.004.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621398#comment-14621398 ] Hadoop QA commented on YARN-3836: - \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:red}-1{color} | pre-patch | 18m 1s | Findbugs (version ) appears to be broken on YARN-2928. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 1 new or modified test files. | | {color:green}+1{color} | javac | 8m 5s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 10m 15s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 23s | The applied patch does not increase the total number of release audit warnings. | | {color:green}+1{color} | checkstyle | 1m 11s | There were no new checkstyle issues. | | {color:green}+1{color} | whitespace | 0m 1s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 42s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 41s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 3m 13s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | yarn tests | 0m 21s | Tests passed in hadoop-yarn-api. | | {color:green}+1{color} | yarn tests | 2m 1s | Tests passed in hadoop-yarn-common. | | | | 46m 0s | | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12744578/YARN-3836-YARN-2928.004.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | YARN-2928 / 4c5f88f | | hadoop-yarn-api test log | https://builds.apache.org/job/PreCommit-YARN-Build/8485/artifact/patchprocess/testrun_hadoop-yarn-api.txt | | hadoop-yarn-common test log | https://builds.apache.org/job/PreCommit-YARN-Build/8485/artifact/patchprocess/testrun_hadoop-yarn-common.txt | | Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/8485/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf904.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux | | Console output | https://builds.apache.org/job/PreCommit-YARN-Build/8485/console | This message was automatically generated. > 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, YARN-3836-YARN-2928.003.patch, > YARN-3836-YARN-2928.004.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621338#comment-14621338 ] Hadoop QA commented on YARN-3836: - \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:red}-1{color} | pre-patch | 19m 37s | Findbugs (version ) appears to be broken on YARN-2928. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 1 new or modified test files. | | {color:green}+1{color} | javac | 8m 2s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 9m 54s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 24s | The applied patch does not increase the total number of release audit warnings. | | {color:green}+1{color} | checkstyle | 1m 7s | There were no new checkstyle issues. | | {color:green}+1{color} | whitespace | 0m 1s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 38s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 42s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 3m 4s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | yarn tests | 0m 22s | Tests passed in hadoop-yarn-api. | | {color:green}+1{color} | yarn tests | 1m 58s | Tests passed in hadoop-yarn-common. | | | | 46m 55s | | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12744556/YARN-3836-YARN-2928.003.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | YARN-2928 / 4c5f88f | | hadoop-yarn-api test log | https://builds.apache.org/job/PreCommit-YARN-Build/8484/artifact/patchprocess/testrun_hadoop-yarn-api.txt | | hadoop-yarn-common test log | https://builds.apache.org/job/PreCommit-YARN-Build/8484/artifact/patchprocess/testrun_hadoop-yarn-common.txt | | Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/8484/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux | | Console output | https://builds.apache.org/job/PreCommit-YARN-Build/8484/console | This message was automatically generated. > 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, YARN-3836-YARN-2928.003.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621288#comment-14621288 ] Sangjin Lee commented on YARN-3836: --- Understood. Somehow I got the impression that the initial patch had the timestamp as the first sort order (which was an incorrect assumption). (TimelineEntity.java) - l.538-539: this can be removed too > 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, YARN-3836-YARN-2928.003.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621209#comment-14621209 ] Zhijie Shen commented on YARN-3836: --- bq. 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(). Currently getEntities supports only return the entities of a single entity type, such that the ordering among them won't be affected by the entity type. In general, it's seem to be more natural to put entities of the same type close to each other. For example, we can merge to the collection of entities returned from multiple getEntities queries to imitate fetching entities of multiple entity types. In case that we have the specific use case (e.g., we want to order entities globally across type), it should be fine and not expensive to define a customized comparator to do it. > 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, YARN-3836-YARN-2928.003.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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621033#comment-14621033 ] Sangjin Lee commented on YARN-3836: --- I take back my comment about the null check for {{getIdentifier()}}. Looking at it, I see that {{getIdentifier()}} will never return null. Sorry for the confusion. > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14620868#comment-14620868 ] Sangjin Lee commented on YARN-3836: --- I tend to think that using type + id is probably a slightly better idea. Currently the type is between single data vs. time series. For the most part, the id should be unique across the board. One interesting scenario is if a metric changes from a single data to a time series (or vice versa). Again, this is probably not something that should happen often, if ever. But if it should happen, I happen to think that they need to be considered two different metrics. My 2 cents. > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14620861#comment-14620861 ] Li Lu commented on YARN-3836: - bq. Regarding metric, can't id uniquely identify a metric ? Do we expect two metrics to share same id for different types ? This is a tricky point. and I'm thinking out loud... Under normal circumstances it's fine to only check the id of metrics. However, since we're making different assumptions on the internal data of different types, is it possible that under some use cases users may mistakenly or accidentally confuse them? If this is possible we may want to check both types and ids. > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14620049#comment-14620049 ] Hadoop QA commented on YARN-3836: - \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:red}-1{color} | pre-patch | 16m 49s | Findbugs (version ) appears to be broken on YARN-2928. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 1 new or modified test files. | | {color:green}+1{color} | javac | 7m 44s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 9m 43s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 23s | The applied patch does not increase the total number of release audit warnings. | | {color:green}+1{color} | checkstyle | 1m 6s | There were no new checkstyle issues. | | {color:green}+1{color} | whitespace | 0m 1s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 39s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 39s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 3m 4s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | yarn tests | 0m 22s | Tests passed in hadoop-yarn-api. | | {color:green}+1{color} | yarn tests | 1m 57s | Tests passed in hadoop-yarn-common. | | | | 43m 33s | | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12744429/YARN-3836-YARN-2928.002.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | YARN-2928 / 4c5f88f | | hadoop-yarn-api test log | https://builds.apache.org/job/PreCommit-YARN-Build/8469/artifact/patchprocess/testrun_hadoop-yarn-api.txt | | hadoop-yarn-common test log | https://builds.apache.org/job/PreCommit-YARN-Build/8469/artifact/patchprocess/testrun_hadoop-yarn-common.txt | | Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/8469/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux | | Console output | https://builds.apache.org/job/PreCommit-YARN-Build/8469/console | This message was automatically generated. > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14620045#comment-14620045 ] Varun Saxena commented on YARN-3836: Regarding metric, can't id uniquely identify a metric ? Do we expect two metrics to share same id for different types ? > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619729#comment-14619729 ] Zhijie Shen commented on YARN-3836: --- bq. I see that we're implementing the Comparable interface for all 3 types. I'm wondering if it makes sense for them. What would it mean to order TimelineEntity instances? Does it mean much? Where would it be useful? Do we need to implement it? The same questions go for the other 2 types... For example, compareTo of TimelineEntity is used to order the entities in the return set of getEntities query. It would be better to return the entities ordered by timestamp instead of randomly. bq. his is an open question. Is the id alone the identity or does the timestamp together form the identity? Do we expect users of TimelineEvent always be able to provide the timestamp? Honestly I'm not 100% sure what the contract is, and we probably want to make it explicit (and add it to the javadoc). Thoughts? In ATS v1, we actually use id + timestamp to uniquely identify an event. On merit of doing this is to let the app to put the same event multiple times. For example, a job can request resource many times. Every time it can put a RESOURCE_REQUEST event with a unique timestamp and fill in the resource information. > 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 > > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619702#comment-14619702 ] Sangjin Lee commented on YARN-3836: --- Thanks [~gtCarrera9] for your quick patch! I agree mostly with your 2 points above. I also did take a quick look at the patch, and here are my initial comments. I see that we're implementing the {{Comparable}} interface for all 3 types. I'm wondering if it makes sense for them. What would it mean to order {{TimelineEntity}} instances? Does it mean much? Where would it be useful? Do we need to implement it? The same questions go for the other 2 types... (TimelineEntity.java) What I would prefer is to override {{equals()}} and {{hashCode()}} for {{Identifier}} instead, and have simple {{equals()}} and {{hashCode()}} implementations for {{TimelineEntity}} that mostly delegate to {{Identifier}}. The rationale is that {{Identifier}} can be useful as keys to collections in its own right, and thus should override those methods. One related question for your use case of putting entities into a map: I notice that you're using the {{TimelineEntity}} instances directly as keys to maps. Wouldn't it be better to use their {{Identifier}} instances as keys instead? {{Identifier}} instances are easier and cheaper to construct and compare. We still need {{equals()}} and {{hashCode()}} on {{TimelineEntity}} itself because they can be used in sets too. - l.42: We should make {{isValid()}} a proper javadoc hyperlink - l.510: Although this is probably going to be true for the most part, this check is a little bit stronger than I expected. We're essentially saying the actual class types of two objects must match precisely. People might extend classes further. Since we're checking the entity type and the id, wouldn't it be sufficient to check whether the object is an instance of {{TimelineEntity}}? (TimelineEvent.java) This is an open question. Is the id alone the identity or does the timestamp together form the identity? Do we expect users of {{TimelineEvent}} always be able to provide the timestamp? Honestly I'm not 100% sure what the contract is, and we probably want to make it explicit (and add it to the javadoc). Thoughts? - l.100: same comment on the class as above (TimelineMetric.java) - l.144: same comment on the class as above > 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 > > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619678#comment-14619678 ] Hadoop QA commented on YARN-3836: - \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:red}-1{color} | pre-patch | 17m 30s | Findbugs (version ) appears to be broken on YARN-2928. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 1 new or modified test files. | | {color:green}+1{color} | javac | 8m 31s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 10m 43s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 25s | The applied patch does not increase the total number of release audit warnings. | | {color:green}+1{color} | checkstyle | 0m 51s | There were no new checkstyle issues. | | {color:green}+1{color} | whitespace | 0m 0s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 49s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 45s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 3m 24s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | yarn tests | 0m 24s | Tests passed in hadoop-yarn-api. | | {color:green}+1{color} | yarn tests | 2m 6s | Tests passed in hadoop-yarn-common. | | | | 46m 32s | | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12744367/YARN-3836-YARN-2928.001.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | YARN-2928 / 4c5f88f | | hadoop-yarn-api test log | https://builds.apache.org/job/PreCommit-YARN-Build/8464/artifact/patchprocess/testrun_hadoop-yarn-api.txt | | hadoop-yarn-common test log | https://builds.apache.org/job/PreCommit-YARN-Build/8464/artifact/patchprocess/testrun_hadoop-yarn-common.txt | | Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/8464/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf902.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux | | Console output | https://builds.apache.org/job/PreCommit-YARN-Build/8464/console | This message was automatically generated. > 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 > > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619245#comment-14619245 ] Li Lu commented on YARN-3836: - Thanks [~vrushalic]! > 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 > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619145#comment-14619145 ] Vrushali C commented on YARN-3836: -- Hi [~gtCarrera] Sounds good, will reassign to you. thanks Vrushali > 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: Vrushali C > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619140#comment-14619140 ] Li Lu commented on YARN-3836: - Hi [~vrushalic], I'd like to check the progress of this JIRA. Currently I'm blocked by this when building time-based aggregations. If you have any bandwidth problems maybe I can take this over? Thanks! > 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: Vrushali C > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14594416#comment-14594416 ] Vrushali C commented on YARN-3836: -- Alright, sounds good. Will do the above.. > 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: Vrushali C > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14594413#comment-14594413 ] Joep Rottinghuis commented on YARN-3836: [~vrushalic] when you're tackling TimelineEntity anyway, it may be good to add a few items to javadoc: a) The class itself should explain that it is not threadsafe and should be used in the context of a single thread only b) Methods such as getIdentifier(), setIdentifier(), getConfigs(), getInfo() leak their internal implementation. They either take or return mutable objects that when modified mess up the state of the the TimelineEntity. Given the performance implications of making copies for safety, it is probably reasonable to stay as-is, but we should add warnings that callers are not to mess with the given or returned collections and Identifier object (nor to re-use them for other any other purposes) or else the internal state of TimelineEntity will be messed up. When looking at the defaults, the members eagerly initialize with collections, but setters do not verify nulls. Given the lack of javadoc, the api is not clear whether nulls should be not allowed for setters and should not be expected for getters, or if nulls will be possible. We'll probably have to check with the Jackson/Jaxb code if it can properly handle nulls or not. Either we allow nulls or not. Currently HBaseTimelineWriter does not handle nulls. If we don't allow them, I think there should be a null check in the setters. If we do (will be best for performance I think) then clients need to do null checks. Either way we should document which one it is imho. > 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: Vrushali C > > 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)
[jira] [Commented] (YARN-3836) add equals and hashCode to TimelineEntity and other classes in the data model
[ https://issues.apache.org/jira/browse/YARN-3836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14594229#comment-14594229 ] Vrushali C commented on YARN-3836: -- I'll take this up unless someone else wants to. > 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: Vrushali C > > 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)