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

MR job should only set tracking url if history was successfully written

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-beta1, 2.8.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently the RMCommunicator will set the tracking url during unregistration once a job has finished, regardless of whether it actually wrote history or not. If the write to history failed for whatever reason, we should leave the tracking url as null so that we get redirected to the AHS instead of getting a job not found on the JHS.

      1. MAPREDUCE-6927.001.patch
        10 kB
        Eric Badger
      2. MAPREDUCE-6927.002.patch
        12 kB
        Eric Badger
      3. MAPREDUCE-6927.003.patch
        16 kB
        Eric Badger
      4. MAPREDUCE-6927.004.patch
        15 kB
        Eric Badger

        Activity

        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        0 mvndep 0m 8s Maven dependency ordering for branch
        +1 mvninstall 14m 34s trunk passed
        +1 compile 1m 47s trunk passed
        +1 checkstyle 0m 29s trunk passed
        +1 mvnsite 0m 50s trunk passed
        +1 mvneclipse 0m 36s trunk passed
        +1 findbugs 1m 8s trunk passed
        +1 javadoc 0m 30s trunk passed
        0 mvndep 0m 7s Maven dependency ordering for patch
        +1 mvninstall 0m 44s the patch passed
        +1 compile 1m 40s the patch passed
        +1 javac 1m 40s the patch passed
        +1 checkstyle 0m 27s the patch passed
        +1 mvnsite 0m 45s the patch passed
        +1 mvneclipse 0m 31s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 17s the patch passed
        +1 javadoc 0m 26s the patch passed
        +1 unit 9m 9s hadoop-mapreduce-client-app in the patch passed.
        +1 unit 3m 14s hadoop-mapreduce-client-hs in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        39m 51s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue MAPREDUCE-6927
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880066/MAPREDUCE-6927.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ff869078caaa 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 4889913
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7039/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7039/console
        Powered by Apache Yetus 0.4.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 14m 34s trunk passed +1 compile 1m 47s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 36s trunk passed +1 findbugs 1m 8s trunk passed +1 javadoc 0m 30s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 44s the patch passed +1 compile 1m 40s the patch passed +1 javac 1m 40s the patch passed +1 checkstyle 0m 27s the patch passed +1 mvnsite 0m 45s the patch passed +1 mvneclipse 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 17s the patch passed +1 javadoc 0m 26s the patch passed +1 unit 9m 9s hadoop-mapreduce-client-app in the patch passed. +1 unit 3m 14s hadoop-mapreduce-client-hs in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 39m 51s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue MAPREDUCE-6927 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880066/MAPREDUCE-6927.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ff869078caaa 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4889913 Default Java 1.8.0_131 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7039/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7039/console Powered by Apache Yetus 0.4.0 http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the report and patch! It would be helpful if you elaborated a bit more in the JIRA description about the problem this is fixing and how it manifests in practice.

        I think we should set the tracking URL as soon as the .jhist file is successfully put in the done folder which is a bit earlier than where it is now in the patch. Even if we fail to get the conf to done, there should be some MapReduce-specific history once the .jhist file is there. Also as written in the patch, there could be a case where we "successfully" complete processDoneFiles but nothing was actually placed in the done directory (i.e.: for whatever reason mi.getHistoryFile() == null).

        It would be nice if the unit test verified the job history URL was correctly rather than just any string at all. I think the test should also verify that even if the job history event handler receives a job finished event but is unable to complete moving the history to the done directory that it does not set the tracking URL.

        Show
        jlowe Jason Lowe added a comment - Thanks for the report and patch! It would be helpful if you elaborated a bit more in the JIRA description about the problem this is fixing and how it manifests in practice. I think we should set the tracking URL as soon as the .jhist file is successfully put in the done folder which is a bit earlier than where it is now in the patch. Even if we fail to get the conf to done, there should be some MapReduce-specific history once the .jhist file is there. Also as written in the patch, there could be a case where we "successfully" complete processDoneFiles but nothing was actually placed in the done directory (i.e.: for whatever reason mi.getHistoryFile() == null). It would be nice if the unit test verified the job history URL was correctly rather than just any string at all. I think the test should also verify that even if the job history event handler receives a job finished event but is unable to complete moving the history to the done directory that it does not set the tracking URL.
        Hide
        ebadger Eric Badger added a comment -

        Jason Lowe, thanks for the comments. I'm not quite sure why I didn't write a description. I'll fix that.

        I think we should set the tracking URL as soon as the .jhist file is successfully put in the done folder which is a bit earlier than where it is now in the patch.

        I moved the setHistoryFile() call up to just after the .jhist file is moved to done.

        It would be nice if the unit test verified the job history URL was correctly rather than just any string at all.

        Fixed

        I think the test should also verify that even if the job history event handler receives a job finished event but is unable to complete moving the history to the done directory that it does not set the tracking URL.

        I had to mock up more stuff than I wanted to to get this to work. Let me know if you have a better way in mind that can accomplish the same thing.

        Show
        ebadger Eric Badger added a comment - Jason Lowe , thanks for the comments. I'm not quite sure why I didn't write a description. I'll fix that. I think we should set the tracking URL as soon as the .jhist file is successfully put in the done folder which is a bit earlier than where it is now in the patch. I moved the setHistoryFile() call up to just after the .jhist file is moved to done. It would be nice if the unit test verified the job history URL was correctly rather than just any string at all. Fixed I think the test should also verify that even if the job history event handler receives a job finished event but is unable to complete moving the history to the done directory that it does not set the tracking URL. I had to mock up more stuff than I wanted to to get this to work. Let me know if you have a better way in mind that can accomplish the same thing.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 41s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
              trunk Compile Tests
        0 mvndep 0m 16s Maven dependency ordering for branch
        +1 mvninstall 13m 37s trunk passed
        +1 compile 1m 38s trunk passed
        +1 checkstyle 0m 35s trunk passed
        +1 mvnsite 0m 47s trunk passed
        +1 findbugs 1m 8s trunk passed
        +1 javadoc 0m 27s trunk passed
              Patch Compile Tests
        0 mvndep 0m 6s Maven dependency ordering for patch
        +1 mvninstall 0m 40s the patch passed
        +1 compile 1m 37s the patch passed
        +1 javac 1m 37s the patch passed
        -1 checkstyle 0m 30s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 10 new + 527 unchanged - 0 fixed = 537 total (was 527)
        +1 mvnsite 0m 43s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 16s the patch passed
        +1 javadoc 0m 24s the patch passed
              Other Tests
        +1 unit 9m 10s hadoop-mapreduce-client-app in the patch passed.
        +1 unit 3m 19s hadoop-mapreduce-client-hs in the patch passed.
        +1 asflicense 0m 14s The patch does not generate ASF License warnings.
        37m 57s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue MAPREDUCE-6927
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880419/MAPREDUCE-6927.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux b272468d7373 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 02bf328
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/console
        Powered by Apache Yetus 0.5.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 41s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.       trunk Compile Tests 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 13m 37s trunk passed +1 compile 1m 38s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 0m 47s trunk passed +1 findbugs 1m 8s trunk passed +1 javadoc 0m 27s trunk passed       Patch Compile Tests 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 0m 40s the patch passed +1 compile 1m 37s the patch passed +1 javac 1m 37s the patch passed -1 checkstyle 0m 30s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 10 new + 527 unchanged - 0 fixed = 537 total (was 527) +1 mvnsite 0m 43s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 16s the patch passed +1 javadoc 0m 24s the patch passed       Other Tests +1 unit 9m 10s hadoop-mapreduce-client-app in the patch passed. +1 unit 3m 19s hadoop-mapreduce-client-hs in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 37m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue MAPREDUCE-6927 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880419/MAPREDUCE-6927.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b272468d7373 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 02bf328 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7044/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for updating the patch!

        Technically moveToDoneNow can end up failing silently if the staging directory does not exist. moveToDoneNow could return a boolean result so the caller can know whether the move actually was performed.

        For the test it would be better to split them into two separate tests, otherwise we're making assumptions that sending two job finished events is something the job history event handler expects and handles properly, which isn't something it would normally see in practice.

        I had to mock up more stuff than I wanted to to get this to work.

        Another way to approach the test is to get the moveToDoneNow method to throw an IOException, and that's going to test the most likely error to happen with the code rather than clearing the fileMap which isn't a likely scenario. moveToDoneNow could be upgraded from private to protected so the test class can override the behavior to always throw for that test.

        It would be good to address the checkstyle issues as well.

        Show
        jlowe Jason Lowe added a comment - Thanks for updating the patch! Technically moveToDoneNow can end up failing silently if the staging directory does not exist. moveToDoneNow could return a boolean result so the caller can know whether the move actually was performed. For the test it would be better to split them into two separate tests, otherwise we're making assumptions that sending two job finished events is something the job history event handler expects and handles properly, which isn't something it would normally see in practice. I had to mock up more stuff than I wanted to to get this to work. Another way to approach the test is to get the moveToDoneNow method to throw an IOException, and that's going to test the most likely error to happen with the code rather than clearing the fileMap which isn't a likely scenario. moveToDoneNow could be upgraded from private to protected so the test class can override the behavior to always throw for that test. It would be good to address the checkstyle issues as well.
        Hide
        ebadger Eric Badger added a comment -

        Jason Lowe, that all makes sense. Definitely a better way to do the tests than I was before. I've fixed them up and separated them. I also fixed the checkstyle, so hopefully that comes back clean.

        Show
        ebadger Eric Badger added a comment - Jason Lowe , that all makes sense. Definitely a better way to do the tests than I was before. I've fixed them up and separated them. I also fixed the checkstyle, so hopefully that comes back clean.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 2m 4s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
              trunk Compile Tests
        0 mvndep 1m 0s Maven dependency ordering for branch
        +1 mvninstall 14m 46s trunk passed
        +1 compile 1m 50s trunk passed
        +1 checkstyle 0m 37s trunk passed
        +1 mvnsite 0m 57s trunk passed
        +1 findbugs 1m 9s trunk passed
        +1 javadoc 0m 30s trunk passed
              Patch Compile Tests
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 0m 43s the patch passed
        +1 compile 1m 39s the patch passed
        +1 javac 1m 39s the patch passed
        -1 checkstyle 0m 33s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 3 new + 524 unchanged - 2 fixed = 527 total (was 526)
        +1 mvnsite 0m 45s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 15s the patch passed
        +1 javadoc 0m 24s the patch passed
              Other Tests
        +1 unit 8m 50s hadoop-mapreduce-client-app in the patch passed.
        +1 unit 3m 11s hadoop-mapreduce-client-hs in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        41m 27s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue MAPREDUCE-6927
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880468/MAPREDUCE-6927.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 9a5819999dd0 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / f44b349
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/console
        Powered by Apache Yetus 0.5.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 2m 4s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.       trunk Compile Tests 0 mvndep 1m 0s Maven dependency ordering for branch +1 mvninstall 14m 46s trunk passed +1 compile 1m 50s trunk passed +1 checkstyle 0m 37s trunk passed +1 mvnsite 0m 57s trunk passed +1 findbugs 1m 9s trunk passed +1 javadoc 0m 30s trunk passed       Patch Compile Tests 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 43s the patch passed +1 compile 1m 39s the patch passed +1 javac 1m 39s the patch passed -1 checkstyle 0m 33s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 3 new + 524 unchanged - 2 fixed = 527 total (was 526) +1 mvnsite 0m 45s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 15s the patch passed +1 javadoc 0m 24s the patch passed       Other Tests +1 unit 8m 50s hadoop-mapreduce-client-app in the patch passed. +1 unit 3m 11s hadoop-mapreduce-client-hs in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 41m 27s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue MAPREDUCE-6927 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880468/MAPREDUCE-6927.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9a5819999dd0 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f44b349 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7045/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for updating the patch! It's really close.

        The moveTmpToDone changes are frivolous since nobody is checking the return value. I think we can leave those changes out.

        Since this is touching the "copy failed" log message, it would be nice if we updated that message to be more specific on the source and dest involved.

        It would be good to cleanup the unused imports and line lengths.

        Show
        jlowe Jason Lowe added a comment - Thanks for updating the patch! It's really close. The moveTmpToDone changes are frivolous since nobody is checking the return value. I think we can leave those changes out. Since this is touching the "copy failed" log message, it would be nice if we updated that message to be more specific on the source and dest involved. It would be good to cleanup the unused imports and line lengths.
        Hide
        ebadger Eric Badger added a comment -

        Thanks for the additional review. Thought I got rid of all of the checkstyle issues, but I must've added in new ones as I updated the patch. I'm pretty sure this one got rid of them all. I had to move moveTmpToDone to protected so that I could override it in the test. Not sure if there's another way around that.

        Show
        ebadger Eric Badger added a comment - Thanks for the additional review. Thought I got rid of all of the checkstyle issues, but I must've added in new ones as I updated the patch. I'm pretty sure this one got rid of them all. I had to move moveTmpToDone to protected so that I could override it in the test. Not sure if there's another way around that.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
              trunk Compile Tests
        0 mvndep 0m 11s Maven dependency ordering for branch
        +1 mvninstall 16m 15s trunk passed
        +1 compile 2m 8s trunk passed
        +1 checkstyle 0m 38s trunk passed
        +1 mvnsite 0m 59s trunk passed
        +1 findbugs 1m 17s trunk passed
        +1 javadoc 0m 34s trunk passed
              Patch Compile Tests
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 0m 53s the patch passed
        +1 compile 2m 1s the patch passed
        +1 javac 2m 1s the patch passed
        +1 checkstyle 0m 36s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 0 new + 524 unchanged - 2 fixed = 524 total (was 526)
        +1 mvnsite 0m 49s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 28s the patch passed
        +1 javadoc 0m 28s the patch passed
              Other Tests
        +1 unit 9m 32s hadoop-mapreduce-client-app in the patch passed.
        +1 unit 3m 29s hadoop-mapreduce-client-hs in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        42m 57s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue MAPREDUCE-6927
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880692/MAPREDUCE-6927.004.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a038fb958b28 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / adb84f3
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7048/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7048/console
        Powered by Apache Yetus 0.5.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.       trunk Compile Tests 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 16m 15s trunk passed +1 compile 2m 8s trunk passed +1 checkstyle 0m 38s trunk passed +1 mvnsite 0m 59s trunk passed +1 findbugs 1m 17s trunk passed +1 javadoc 0m 34s trunk passed       Patch Compile Tests 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 53s the patch passed +1 compile 2m 1s the patch passed +1 javac 2m 1s the patch passed +1 checkstyle 0m 36s hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 0 new + 524 unchanged - 2 fixed = 524 total (was 526) +1 mvnsite 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 28s the patch passed +1 javadoc 0m 28s the patch passed       Other Tests +1 unit 9m 32s hadoop-mapreduce-client-app in the patch passed. +1 unit 3m 29s hadoop-mapreduce-client-hs in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 42m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue MAPREDUCE-6927 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880692/MAPREDUCE-6927.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a038fb958b28 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / adb84f3 Default Java 1.8.0_131 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7048/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7048/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        +1 lgtm. Committing this.

        Show
        jlowe Jason Lowe added a comment - +1 lgtm. Committing this.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks, Eric! I committed this to trunk, branch-2, branch-2.8, and branch-2.8.2.

        Show
        jlowe Jason Lowe added a comment - Thanks, Eric! I committed this to trunk, branch-2, branch-2.8, and branch-2.8.2.
        Hide
        ebadger Eric Badger added a comment -

        Thanks, Jason Lowe for the review and commit!

        Show
        ebadger Eric Badger added a comment - Thanks, Jason Lowe for the review and commit!

          People

          • Assignee:
            ebadger Eric Badger
            Reporter:
            ebadger Eric Badger
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development