Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-4415

Backport the Job.getInstance methods from MAPREDUCE-1505 to branch-1

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0
    • Fix Version/s: 1.2.0
    • Component/s: mrv1
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Backported new APIs to get a Job object to 1.2.0 from 2.0.0. Job API static methods Job.getInstance(), Job.getInstance(Configuration) and Job.getInstance(Configuration, jobName) are now available across both releases to avoid porting pain.

      Description

      In 2.x MR, the Job constructors have all been deprecated in favor of Job.getInstance() calls to get a Job object.

      However, these getInstance methods do not appear to be present in the 1.x MR API, and thereby may cause additional pain to users moving from 1.x to 2.x going forward.

      This patch proposes to add in the getInstance style of methods with suitable test coverage for both style of constructors, while not pulling in anything else from MAPREDUCE-1505 (as we lack 'Cluster' in 1.x). As we're not going to be deprecating the regular ctors in a 1.x release, this is not an incompatible change in any way.

        Activity

        Hide
        qwertymaniac Harsh J added a comment -

        A backport of 3 static methods with javadocs, and a smattering of test changes to cover them (keeps-unchanged adequate older-style constructor tests, to preserve coverage).

        Show
        qwertymaniac Harsh J added a comment - A backport of 3 static methods with javadocs, and a smattering of test changes to cover them (keeps-unchanged adequate older-style constructor tests, to preserve coverage).
        Hide
        qwertymaniac Harsh J added a comment -

        Running a test-patch right now, will post results shortly.

        Show
        qwertymaniac Harsh J added a comment - Running a test-patch right now, will post results shortly.
        Hide
        qwertymaniac Harsh J added a comment -

        test-patch results:

             [exec] -1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     +1 tests included.  The patch appears to include 12 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     -1 findbugs.  The patch appears to introduce 218 new Findbugs (version 2.0.1-rc3) warnings.
        
        Show
        qwertymaniac Harsh J added a comment - test-patch results: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 12 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] -1 findbugs. The patch appears to introduce 218 new Findbugs (version 2.0.1-rc3) warnings.
        Hide
        qwertymaniac Harsh J added a comment -

        -1 findbugs. The patch appears to introduce 218 new Findbugs (version 2.0.1-rc3) warnings.

        Same count as the one I received on HDFS-3617 earlier today. None of these findbugs are caused by my code. Hence that -1 can be ignored for this patch.

        Show
        qwertymaniac Harsh J added a comment - -1 findbugs. The patch appears to introduce 218 new Findbugs (version 2.0.1-rc3) warnings. Same count as the one I received on HDFS-3617 earlier today. None of these findbugs are caused by my code. Hence that -1 can be ignored for this patch.
        Hide
        acmurthy Arun C Murthy added a comment -

        Harsh - there are many more instances of 'new Job()' ?

        Do you plan to fix all of them?

        Show
        acmurthy Arun C Murthy added a comment - Harsh - there are many more instances of 'new Job()' ? Do you plan to fix all of them?
        Hide
        qwertymaniac Harsh J added a comment -

        Arun - I thought I'd leave some to not mess with test coverage for that ctor, as it has not been deprecated/replaced by this change. If you feel we should change it all, I'll go ahead. Let me know!

        Show
        qwertymaniac Harsh J added a comment - Arun - I thought I'd leave some to not mess with test coverage for that ctor, as it has not been deprecated/replaced by this change. If you feel we should change it all, I'll go ahead. Let me know!
        Hide
        qwertymaniac Harsh J added a comment -

        Arun - Ping?

        Show
        qwertymaniac Harsh J added a comment - Arun - Ping?
        Hide
        tomwhite Tom White added a comment -

        +1 looks good to me.

        Show
        tomwhite Tom White added a comment - +1 looks good to me.
        Hide
        qwertymaniac Harsh J added a comment -

        Committing to branch-1, thanks Tom and Arun!

        Show
        qwertymaniac Harsh J added a comment - Committing to branch-1, thanks Tom and Arun!
        Hide
        qwertymaniac Harsh J added a comment -

        (Committed to branch-1 via r1365193)

        Show
        qwertymaniac Harsh J added a comment - (Committed to branch-1 via r1365193)
        Hide
        mattf Matt Foley added a comment -

        Closed upon release of Hadoop 1.2.0.

        Show
        mattf Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.

          People

          • Assignee:
            qwertymaniac Harsh J
            Reporter:
            qwertymaniac Harsh J
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development