[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576521#comment-13576521 ] nkeywal commented on HBASE-7789: Committed (v2) thanks for the review, Ted! Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch, 7789.v2.patch I need to do some changes in DeadServer because of HBASE-7590. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7590, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576537#comment-13576537 ] Hadoop QA commented on HBASE-7789: -- {color:green}+1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12568947/7789.v2.patch against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 9 new or modified tests. {color:green}+1 hadoop2.0{color}. The patch compiles against the hadoop 2.0 profile. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 core tests{color}. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4421//console This message is automatically generated. Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch, 7789.v2.patch I need to do some changes in DeadServer because of HBASE-7590. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7590, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576573#comment-13576573 ] Hudson commented on HBASE-7789: --- Integrated in HBase-TRUNK #3870 (See [https://builds.apache.org/job/HBase-TRUNK/3870/]) HBASE-7789 Clean DeadServer.java and add a Jitter method in ConnectionUtils (Revision 1445087) Result = FAILURE nkeywal : Files : * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterStatusServlet.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch, 7789.v2.patch I need to do some changes in DeadServer because of HBASE-7590. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7590, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13576583#comment-13576583 ] Hudson commented on HBASE-7789: --- Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #404 (See [https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/404/]) HBASE-7789 Clean DeadServer.java and add a Jitter method in ConnectionUtils (Revision 1445087) Result = FAILURE nkeywal : Files : * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterStatusServlet.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java * /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java * /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch, 7789.v2.patch I need to do some changes in DeadServer because of HBASE-7590. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7590, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13575692#comment-13575692 ] nkeywal commented on HBASE-7789: Ok for the comment, will do that in the commit (tomorrow) if there are not other reviews. Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13574311#comment-13574311 ] Elliott Clark commented on HBASE-7789: -- bq.for example to be shown in the UI. That would be awesome. Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13574362#comment-13574362 ] nkeywal commented on HBASE-7789: The line in TestAssignment took me longer than expected... Local tests seems ok (except flakiness) @stack The advantage of returning the object instead of a clone of the internal list is that stuff like return serverManager.getDeadServers().size(); (in MetricsMasterWrapperImpl) does not do a (hidden) copy anymore. As the method is synchronized, it's not necessary (and clone was already synchronized, so we have less synchronization now). Agreed, the list size is usually not huge, but still... Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13574390#comment-13574390 ] Hadoop QA commented on HBASE-7789: -- {color:green}+1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12568550/7789.v1.patch against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 9 new or modified tests. {color:green}+1 hadoop2.0{color}. The patch compiles against the hadoop 2.0 profile. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 lineLengths{color}. The patch does not introduce lines longer than 100 {color:green}+1 core tests{color}. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4382//console This message is automatically generated. Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13574692#comment-13574692 ] Ted Yu commented on HBASE-7789: --- {code} + * Adds / subs a 10% jitter. Minimum is 1. + */ + public static long addJitter(final long pause, final float jitter) { {code} Since the above is a public method, please add javadoc for the parameters. {code} + return 1; +} else { {code} 'else' is not needed. {code} + * @return a sorted array list, by death time. {code} Please add sort order description. {code} + private static ComparatorPairServerName, Long ServerNameDateComparator = ... +public int compare(PairServerName, Long o1, PairServerName, Long o2) { + return o1.getSecond().compareTo(o2.getSecond()); {code} Here only the time is compared. Maybe there is a better name for the comparator class ? Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 Attachments: 7789.v1.patch I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-7789) Clean DeadServer.java and add a Jitter method in ConnectionUtils
[ https://issues.apache.org/jira/browse/HBASE-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13573737#comment-13573737 ] stack commented on HBASE-7789: -- Sounds good except for 'use directly the object instead of a copy.' Whats wrong w/ making new instance if something changes? Don't we only make these once? Clean DeadServer.java and add a Jitter method in ConnectionUtils Key: HBASE-7789 URL: https://issues.apache.org/jira/browse/HBASE-7789 Project: HBase Issue Type: Bug Components: master Affects Versions: 0.96.0 Reporter: nkeywal Assignee: nkeywal Fix For: 0.96.0 I need to do some changes in DeadServer because of HBASE-7390. To minimize the patch size and simplifies the feedback, I prefer to isolate the issue. Changes are: - Add the time when the server was declared as dead. It's what I need in HBASE-7390, but it makes sense even without it, for example to be shown in the UI. - suppress the extends on Set clean up all the not used methods - use directly the object instead of a copy. For connection utils, we currently have a jitter of 1%. I need a bigger one for sure in one case, but I wonder if we should not increase it in all cases? instead of plus 1%, we should have plus or minus 10% imho. Tests are in progress locally, I will add the patch when they're ok. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira