Hadoop Common
  1. Hadoop Common
  2. HADOOP-7788

HA: Simple HealthMonitor class to watch an HAService

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.24.0
    • Fix Version/s: 0.23.4
    • Component/s: ha
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      This is a utility class which will be part of the FailoverController. The class starts a daemon thread which periodically monitors an HAService, calling its monitorHealth function. It then generates callbacks into another class when the health status changes (eg the RPC fails or the service returns a HealthCheckFailedException)

      1. hdfs-2524.txt
        15 kB
        Todd Lipcon
      2. hadoop-7788.txt
        18 kB
        Todd Lipcon
      3. hadoop-7788.txt
        21 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1026 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1026/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1026 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1026/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Build #232 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/232/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207)

          Result = FAILURE
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #232 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/232/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #204 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/204/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #204 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/204/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #991 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/991/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208)

          Result = UNSTABLE
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #991 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/991/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208) Result = UNSTABLE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #1982 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1982/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1982 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1982/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-0.23-Commit #709 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/709/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #709 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/709/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1908 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1908/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1908 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1908/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Commit #700 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/700/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207)

          Result = SUCCESS
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #700 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/700/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1917 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1917/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208)

          Result = ABORTED
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1917 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1917/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303208) Result = ABORTED todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303208 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Commit #717 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/717/)
          HADOOP-7788. Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207)

          Result = ABORTED
          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #717 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/717/ ) HADOOP-7788 . Add simple HealthMonitor class to watch an HAService. Contributed by Todd Lipcon. (Revision 1303207) Result = ABORTED todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1303207 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/protocolPB/HAServiceProtocolClientSideTranslatorPB.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHealthMonitor.java
          Hide
          Todd Lipcon added a comment -

          Committed to trunk and branch-23. thanks for reviewing, ATM.

          Show
          Todd Lipcon added a comment - Committed to trunk and branch-23. thanks for reviewing, ATM.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12519015/hadoop-7788.txt
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/737//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/737//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12519015/hadoop-7788.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/737//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/737//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          +1, the latest patch looks good to me.

          Show
          Aaron T. Myers added a comment - +1, the latest patch looks good to me.
          Hide
          Todd Lipcon added a comment -

          New rev of the patch:

          • fixed the shouldRun = false thing
          • I moved the daemon to be an internal member of the class, because looking at the code again I remembered this Java bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6609468 which can cause a deadlock if you synchronize on a Thread object ever. Since this class uses synchronization on itself, I think it's better to not be a Thread.
          • added a new "FAILED" state as per one of the TODOs in the earlier patch. This gets set if the daemon itself fails for any reason
          • separated out the error handling of the case where we get an exception talking to the monitored service, from the case where the callback itself fails. In the case where the service is the one causing the exception, I am not printing the full stack trace since it's a bit verbose and the message is generally descriptive enough.
          Show
          Todd Lipcon added a comment - New rev of the patch: fixed the shouldRun = false thing I moved the daemon to be an internal member of the class, because looking at the code again I remembered this Java bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6609468 which can cause a deadlock if you synchronize on a Thread object ever. Since this class uses synchronization on itself, I think it's better to not be a Thread. added a new "FAILED" state as per one of the TODOs in the earlier patch. This gets set if the daemon itself fails for any reason separated out the error handling of the case where we get an exception talking to the monitored service, from the case where the callback itself fails. In the case where the service is the one causing the exception, I am not printing the full stack trace since it's a bit verbose and the message is generally descriptive enough.
          Hide
          Aaron T. Myers added a comment -

          Largely looks good, Todd. A few small comments:

          1. I don't understand why it's necessary/desirable to set "shouldRun = false" in the case of an InterruptedException in run(). When would this method be interrupted except in the case of a call to shutdown(), which itself sets shouldRun to false?
          2. "LOG.warn("Transport-level exception trying to monitor health of " + addrToMonitor + ": " + t.getLocalizedMessage());" - let's log the full exception stack trace here.
          3. Seems odd to me that the main method of this class creates a HealthMonitor and calls run(), instead of calling start()/join().

          +1 once these are addressed.

          Show
          Aaron T. Myers added a comment - Largely looks good, Todd. A few small comments: I don't understand why it's necessary/desirable to set " shouldRun = false " in the case of an InterruptedException in run() . When would this method be interrupted except in the case of a call to shutdown() , which itself sets shouldRun to false? " LOG.warn("Transport-level exception trying to monitor health of " + addrToMonitor + ": " + t.getLocalizedMessage()); " - let's log the full exception stack trace here. Seems odd to me that the main method of this class creates a HealthMonitor and calls run() , instead of calling start() / join() . +1 once these are addressed.
          Hide
          Todd Lipcon added a comment -

          Oh, sorry, I also left in the main() method. Though the test covers the code fairly well, having a main() method is helpful for manual testing of some things like kill -STOPping the monitored process and making sure timeouts are handled correctly, etc. That's hard to mock out.

          Show
          Todd Lipcon added a comment - Oh, sorry, I also left in the main() method. Though the test covers the code fairly well, having a main() method is helpful for manual testing of some things like kill -STOPping the monitored process and making sure timeouts are handled correctly, etc. That's hard to mock out.
          Hide
          Todd Lipcon added a comment -

          New version of the patch addresses Suresh's feedback above. I also added a getProxy() and getLastServiceState() method, which I needed for the ZK Failover Controller.

          The only feedback I didn't address was to add annotations to HealthMonitor. Since it's a package-private class, I don't think it has to be annotated.

          Show
          Todd Lipcon added a comment - New version of the patch addresses Suresh's feedback above. I also added a getProxy() and getLastServiceState() method, which I needed for the ZK Failover Controller. The only feedback I didn't address was to add annotations to HealthMonitor. Since it's a package-private class, I don't think it has to be annotated.
          Hide
          Aaron T. Myers added a comment -

          Converting to top-level issue with commit of HADOOP-7454.

          Show
          Aaron T. Myers added a comment - Converting to top-level issue with commit of HADOOP-7454 .
          Hide
          Suresh Srinivas added a comment -

          Comments:

          1. SERVICE_UNHEALTHY_NOT_RESPONDING - do we have SERVICE_HEALTHY_NOT_RESPONDING? If not, we should call this SERVICE_NOT_RESPONDING.
          2. Please add javadoc to HealthMonitor class and annotations
          3. Should we document what can be done in callback? Can it take a long time etc.?
          4. In order to keep the interface complete, it may be a good idea to add removeCallBack(). I cannot think of a use for this, right now though.
          5. CHECK_INTERVAL_DEFAULT is 5 seconds - it would be better do this in shorter duration. Say 1s?
          6. With tests added main() method seems unnecessary.
          Show
          Suresh Srinivas added a comment - Comments: SERVICE_UNHEALTHY_NOT_RESPONDING - do we have SERVICE_HEALTHY_NOT_RESPONDING? If not, we should call this SERVICE_NOT_RESPONDING. Please add javadoc to HealthMonitor class and annotations Should we document what can be done in callback? Can it take a long time etc.? In order to keep the interface complete, it may be a good idea to add removeCallBack(). I cannot think of a use for this, right now though. CHECK_INTERVAL_DEFAULT is 5 seconds - it would be better do this in shorter duration. Say 1s? With tests added main() method seems unnecessary.
          Hide
          Suresh Srinivas added a comment -

          Sorry I was not clear in my previous comment - What I meant was is this configuration option and should we move this to some ConfigKeys with default values.

          Show
          Suresh Srinivas added a comment - Sorry I was not clear in my previous comment - What I meant was is this configuration option and should we move this to some ConfigKeys with default values.
          Hide
          Suresh Srinivas added a comment -

          Took a quick look. Can you have constants instead of hardcoded strings. Eg. ha.health-monitor.check-interval.ms

          Will post comments soon.

          Show
          Suresh Srinivas added a comment - Took a quick look. Can you have constants instead of hardcoded strings. Eg. ha.health-monitor.check-interval.ms Will post comments soon.
          Hide
          Eli Collins added a comment -

          Nit, I'd have the first state be SERVICE_HEALTH_UNKNOWN rather than INITIALIZING so the value of state always refers to the state of the service being monitored.

          Otherwise +1, looks good.

          Show
          Eli Collins added a comment - Nit, I'd have the first state be SERVICE_HEALTH_UNKNOWN rather than INITIALIZING so the value of state always refers to the state of the service being monitored. Otherwise +1, looks good.
          Hide
          Todd Lipcon added a comment -

          Here's a basic implementation. It includes a unit test as well as a "main" function which I used to test against a NN.

          Show
          Todd Lipcon added a comment - Here's a basic implementation. It includes a unit test as well as a "main" function which I used to test against a NN.

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development