Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-2337

Remove dependence of public MapReduce API on classes in server package

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Cluster#getJobTrackerState() returns a org.apache.hadoop.mapreduce.server.jobtracker.State enum, which makes the API in o.a.h.mapreduce have a dependency on the server package. It would be better to make the public API self-contained by using an equivalent enum in the Cluster class.

      1. MAPREDUCE-2337.patch
        2 kB
        Tom White
      2. MAPREDUCE-2337.patch
        6 kB
        Tom White
      3. MAPREDUCE-2337.patch
        12 kB
        Tom White
      4. MAPREDUCE-2337.patch
        12 kB
        Tom White

        Issue Links

          Activity

          Hide
          Tom White added a comment -

          Patch which deprecates the method and enum in favour of new ones in Cluster.

          Show
          Tom White added a comment - Patch which deprecates the method and enum in favour of new ones in Cluster.
          Hide
          Tom White added a comment -

          New patch which deprecates the equivalent in the old API too, and changes the tests to use the new method.

          Show
          Tom White added a comment - New patch which deprecates the equivalent in the old API too, and changes the tests to use the new method.
          Hide
          Todd Lipcon added a comment -

          I think this makes sense, but maybe it actually should get pushed into the protocol package? ie the new State enum should be part of ClientProtocol, and ClientProtocol.getJobTrackerStatus() would return the new type while deprecating ClientProtocol.getJobTrackerState? It doesn't seem right that ClientProtocol exposes the server-internal state type to me.

          Show
          Todd Lipcon added a comment - I think this makes sense, but maybe it actually should get pushed into the protocol package? ie the new State enum should be part of ClientProtocol, and ClientProtocol.getJobTrackerStatus() would return the new type while deprecating ClientProtocol.getJobTrackerState? It doesn't seem right that ClientProtocol exposes the server-internal state type to me.
          Hide
          Tom White added a comment -

          It doesn't seem right that ClientProtocol exposes the server-internal state type to me.

          Good point. However I think that ClientProtocol (which is Private) should return a mapreduce.Cluster.JobTrackerStatus, just like its getClusterMetrics() method returns a mapreduce.ClusterMetrics object. I'll create a new patch.

          Show
          Tom White added a comment - It doesn't seem right that ClientProtocol exposes the server-internal state type to me. Good point. However I think that ClientProtocol (which is Private) should return a mapreduce.Cluster.JobTrackerStatus, just like its getClusterMetrics() method returns a mapreduce.ClusterMetrics object. I'll create a new patch.
          Hide
          Tom White added a comment -

          Here's a new patch with the above changes.

          Show
          Tom White added a comment - Here's a new patch with the above changes.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12471866/MAPREDUCE-2337.patch
          against trunk revision 1074251.

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

          +1 tests included. The patch appears to include 12 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 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 failed these core unit tests:
          org.apache.hadoop.mapred.TestControlledMapReduceJob

          -1 contrib tests. The patch failed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//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/12471866/MAPREDUCE-2337.patch against trunk revision 1074251. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 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 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 failed these core unit tests: org.apache.hadoop.mapred.TestControlledMapReduceJob -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/27//console This message is automatically generated.
          Hide
          Tom White added a comment -

          The test failures are unrelated.

          Show
          Tom White added a comment - The test failures are unrelated.
          Hide
          Todd Lipcon added a comment -

          In o.a.h.mapreduce.Cluster#getJobTrackerStatus(), can you delegate that directly to client.getJobTrackerStatus(), rather than calling the now-deprecated getJobTrackerState()?

          Show
          Todd Lipcon added a comment - In o.a.h.mapreduce.Cluster#getJobTrackerStatus(), can you delegate that directly to client.getJobTrackerStatus(), rather than calling the now-deprecated getJobTrackerState()?
          Hide
          Tom White added a comment -

          Fixed. Thanks for the review.

          Show
          Tom White added a comment - Fixed. Thanks for the review.
          Hide
          Todd Lipcon added a comment -

          +1

          Show
          Todd Lipcon added a comment - +1
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12475752/MAPREDUCE-2337.patch
          against trunk revision 1089686.

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

          +1 tests included. The patch appears to include 12 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 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 core unit tests.

          -1 contrib tests. The patch failed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//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/12475752/MAPREDUCE-2337.patch against trunk revision 1089686. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 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 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 core unit tests. -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/158//console This message is automatically generated.
          Hide
          Tom White added a comment -

          I've just committed this.

          Show
          Tom White added a comment - I've just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #639 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/639/)
          MAPREDUCE-2337. Remove dependence of public MapReduce API on classes in server package.

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #639 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/639/ ) MAPREDUCE-2337 . Remove dependence of public MapReduce API on classes in server package.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #39 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/39/)
          Merge -r 1090103:1090104 from trunk to branch-0.22. Fixes: MAPREDUCE-2337

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #39 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/39/ ) Merge -r 1090103:1090104 from trunk to branch-0.22. Fixes: MAPREDUCE-2337
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #646 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/646/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #646 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/646/ )

            People

            • Assignee:
              Tom White
              Reporter:
              Tom White
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development