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

Single shuffle to memory must not exceed Integer#MAX_VALUE

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 2.7.4, 3.0.0-alpha1
    • Component/s: mrv2
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      When shuffle is done in memory, MergeManagerImpl converts the requested size to an int to allocate an instance of InMemoryMapOutput. This results in an overflow if the requested size is bigger than Integer.MAX_VALUE and eventually causes the reducer to fail.

      1. mapreduce6724.001.patch
        8 kB
        Haibo Chen
      2. mapreduce6724.002.patch
        8 kB
        Haibo Chen
      3. mapreduce6724.003.patch
        7 kB
        Haibo Chen
      4. mapreduce6724.004.patch
        7 kB
        Haibo Chen
      5. mapreduce6724.005.patch
        7 kB
        Haibo Chen
      6. mapreduce6724.006.patch
        8 kB
        Haibo Chen
      7. mapreduce6724.007.patch
        4 kB
        Haibo Chen
      8. mapreduce6724.008.patch
        4 kB
        Haibo Chen
      9. MAPREDUCE-6724.009.patch
        5 kB
        Gera Shegalov

        Issue Links

          Activity

          Hide
          jlowe Jason Lowe added a comment -

          Thanks, Haibo Chen! I committed this to branch-2.8 and branch-2.7 as well.

          Show
          jlowe Jason Lowe added a comment - Thanks, Haibo Chen ! I committed this to branch-2.8 and branch-2.7 as well.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #10190 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10190/)
          MAPREDUCE-6724. Single shuffle to memory must not exceed (gera: rev 6890d5b472320fa7592ed1b08b623c55a27089c6)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/task/reduce/MergeManagerImpl.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #10190 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10190/ ) MAPREDUCE-6724 . Single shuffle to memory must not exceed (gera: rev 6890d5b472320fa7592ed1b08b623c55a27089c6) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/task/reduce/MergeManagerImpl.java
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Committed to trunk and branch-2. Thanks Haibo Chen for contribution and to Daniel Templeton for additional reviews.

          Show
          jira.shegalov Gera Shegalov added a comment - Committed to trunk and branch-2. Thanks Haibo Chen for contribution and to Daniel Templeton for additional reviews.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Will commit 009 some time later tomorrow if there are no objections.

          Show
          jira.shegalov Gera Shegalov added a comment - Will commit 009 some time later tomorrow if there are no objections.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 12s trunk passed
          +1 compile 0m 26s trunk passed
          +1 checkstyle 0m 20s trunk passed
          +1 mvnsite 0m 31s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 55s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          +1 checkstyle 0m 18s the patch passed
          +1 mvnsite 0m 31s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 2s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          17m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821233/MAPREDUCE-6724.009.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 851b6ebf1113 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / e5766b1
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6653/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6653/console
          Powered by Apache Yetus 0.3.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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 12s trunk passed +1 compile 0m 26s trunk passed +1 checkstyle 0m 20s trunk passed +1 mvnsite 0m 31s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 55s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 27s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed +1 checkstyle 0m 18s the patch passed +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 17m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821233/MAPREDUCE-6724.009.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 851b6ebf1113 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e5766b1 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6653/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6653/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Adding a slightly modified 009 version on top of 008. I felt a little uneasy about not having a test Integer.MAX_VALUE in-memory output. However, it also requires manipulating Xmx in the surefire plugin config. Some developers might also not have a beefy enough machine to handle such a large allocation. So just making maxSingleShuffleLimit VisibleForTesting.

          Show
          jira.shegalov Gera Shegalov added a comment - Adding a slightly modified 009 version on top of 008. I felt a little uneasy about not having a test Integer.MAX_VALUE in-memory output. However, it also requires manipulating Xmx in the surefire plugin config. Some developers might also not have a beefy enough machine to handle such a large allocation. So just making maxSingleShuffleLimit VisibleForTesting.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 48s trunk passed
          +1 compile 0m 25s trunk passed
          +1 checkstyle 0m 19s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 52s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 24s the patch passed
          +1 compile 0m 21s the patch passed
          +1 javac 0m 21s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 27s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 56s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          16m 27s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820336/mapreduce6724.008.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 75b1ab28eb53 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b43de80
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6645/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6645/console
          Powered by Apache Yetus 0.3.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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 48s trunk passed +1 compile 0m 25s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 52s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 24s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 27s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 56s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 16m 27s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820336/mapreduce6724.008.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 75b1ab28eb53 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b43de80 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6645/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6645/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Hitting "Submit Patch" to get a jenkins run.

          Show
          jira.shegalov Gera Shegalov added a comment - Hitting "Submit Patch" to get a jenkins run.
          Hide
          haibochen Haibo Chen added a comment -

          fixed checkstyle issues.

          Show
          haibochen Haibo Chen added a comment - fixed checkstyle issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 48s trunk passed
          +1 compile 0m 23s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 50s trunk passed
          +1 javadoc 0m 25s trunk passed
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 33s the patch passed
          +1 javac 0m 33s the patch passed
          -1 checkstyle 0m 18s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core: The patch generated 9 new + 101 unchanged - 0 fixed = 110 total (was 101)
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 9s the patch passed
          +1 javadoc 0m 22s the patch passed
          +1 unit 2m 17s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          16m 23s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820315/mapreduce6724.007.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 87e3fa6c9b68 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d84ab8a
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/console
          Powered by Apache Yetus 0.3.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 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 48s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 50s trunk passed +1 javadoc 0m 25s trunk passed +1 mvninstall 0m 27s the patch passed +1 compile 0m 33s the patch passed +1 javac 0m 33s the patch passed -1 checkstyle 0m 18s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core: The patch generated 9 new + 101 unchanged - 0 fixed = 110 total (was 101) +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 9s the patch passed +1 javadoc 0m 22s the patch passed +1 unit 2m 17s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 16m 23s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820315/mapreduce6724.007.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 87e3fa6c9b68 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d84ab8a Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6642/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Uploading a patch as Gera Shegalov suggested instead of bloating the patch with refactoring.

          Show
          haibochen Haibo Chen added a comment - Uploading a patch as Gera Shegalov suggested instead of bloating the patch with refactoring.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Haibo Chen, what I mean if you revert your change to hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java , and apply my shorter change to it, I think it tests the bugs you are fixing: it will fail without the fix, and succeed with the fix.

          Show
          jira.shegalov Gera Shegalov added a comment - Haibo Chen , what I mean if you revert your change to hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java , and apply my shorter change to it, I think it tests the bugs you are fixing: it will fail without the fix, and succeed with the fix.
          Hide
          haibochen Haibo Chen added a comment -

          Gera Shegalov Can you elaborate a little big more on what you mean by existing test covers regression? My understanding of testLargeMemoryLimits() is that it checks memoryLimit and maxInMemReduceLimit, both of which can not be tested with verifyReserveLargeMapOutput(). I could restore changes made to testLargeMemoryLimits to minimize changes to existing tests. By "testing MAX_VALUE", do you mean that we need to have another test case in which singleSHuffleLimit is set to Integer.MAX_VAL?

          Show
          haibochen Haibo Chen added a comment - Gera Shegalov Can you elaborate a little big more on what you mean by existing test covers regression? My understanding of testLargeMemoryLimits() is that it checks memoryLimit and maxInMemReduceLimit, both of which can not be tested with verifyReserveLargeMapOutput(). I could restore changes made to testLargeMemoryLimits to minimize changes to existing tests. By "testing MAX_VALUE", do you mean that we need to have another test case in which singleSHuffleLimit is set to Integer.MAX_VAL?
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Haibo Chen, do you want to keep your test, or do you think my more minor modification (verifyReserveLargeMapOutput) of the existing test covers the regression as well and we can use it instead with the benefit of explicitly testing MAX_VALUE?

          Show
          jira.shegalov Gera Shegalov added a comment - Haibo Chen , do you want to keep your test, or do you think my more minor modification (verifyReserveLargeMapOutput) of the existing test covers the regression as well and we can use it instead with the benefit of explicitly testing MAX_VALUE?
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 29s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 28s trunk passed
          +1 compile 0m 23s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 47s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 21s the patch passed
          +1 javac 0m 21s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 54s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 2m 2s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          16m 8s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819126/mapreduce6724.006.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 4d1d348dfe45 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 38128ba
          Default Java 1.8.0_91
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6630/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6630/console
          Powered by Apache Yetus 0.3.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 29s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 28s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 47s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 54s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 2m 2s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 16m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819126/mapreduce6724.006.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 4d1d348dfe45 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 38128ba Default Java 1.8.0_91 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6630/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6630/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 9s trunk passed
          +1 compile 0m 25s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 52s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 23s the patch passed
          +1 compile 0m 21s the patch passed
          +1 javac 0m 21s the patch passed
          +1 checkstyle 0m 14s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 54s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 5s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          15m 49s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819126/mapreduce6724.006.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 550a593e2212 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 38128ba
          Default Java 1.8.0_91
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6625/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6625/console
          Powered by Apache Yetus 0.3.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 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 9s trunk passed +1 compile 0m 25s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 52s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 23s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed +1 checkstyle 0m 14s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 54s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 5s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 15m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819126/mapreduce6724.006.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 550a593e2212 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 38128ba Default Java 1.8.0_91 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6625/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6625/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Uploading a new patch to have canShuffleToMemory inline. Thanks Gera Shegalov again for reviews.

          Show
          haibochen Haibo Chen added a comment - Uploading a new patch to have canShuffleToMemory inline. Thanks Gera Shegalov again for reviews.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Hi Haibo Chen, thanks for working on the patch. Initially, I probably had had another idea with the same objective of keeping the small. If you and Daniel Templeton agree that the new test version tests what's needed, I'd suggest we keep it.

          Show
          jira.shegalov Gera Shegalov added a comment - Hi Haibo Chen , thanks for working on the patch. Initially, I probably had had another idea with the same objective of keeping the small. If you and Daniel Templeton agree that the new test version tests what's needed, I'd suggest we keep it.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks very much for your comments, Gera Shegalov. I completely misunderstood your previous comments regarding the test refactoring. The reason I believe is that the two properties asserted in testLargeMemoryLimits, memory limit and maxInMemReduceLimit cannot be tested directly with a single call to MerManager.reserver(). In the case of fresh calling reserve() method once, if the requested size is bigger than maxSingleShuffleLimit, then an OnDiskMerger is returned, an InMemMerger otherwise. Thus, I did not think of calling verifyReserveLargeMapOutput() in testLargeMemoryLimits. Let me know if you think otherwise. As for canShuffleToMemory, I'll include it in a newer patch later.

          Show
          haibochen Haibo Chen added a comment - Thanks very much for your comments, Gera Shegalov . I completely misunderstood your previous comments regarding the test refactoring. The reason I believe is that the two properties asserted in testLargeMemoryLimits, memory limit and maxInMemReduceLimit cannot be tested directly with a single call to MerManager.reserver(). In the case of fresh calling reserve() method once, if the requested size is bigger than maxSingleShuffleLimit, then an OnDiskMerger is returned, an InMemMerger otherwise. Thus, I did not think of calling verifyReserveLargeMapOutput() in testLargeMemoryLimits. Let me know if you think otherwise. As for canShuffleToMemory, I'll include it in a newer patch later.
          Hide
          jira.shegalov Gera Shegalov added a comment - - edited

          As for test I was thinking of a smaller change to the existing test along the lines:

          diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java
          index 1c0d25b..5930139 100644
          --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java
          +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java
          @@ -289,6 +289,19 @@ public void testLargeMemoryLimits() throws Exception {
               final long maxInMemReduce = mgr.getMaxInMemReduceLimit();
               assertTrue("Large in-memory reduce area unusable: " + maxInMemReduce,
                   maxInMemReduce > Integer.MAX_VALUE);
          +
          +    // verifyReserveLargeMapOutput(mgr, (long)Integer.MAX_VALUE);
          +    verifyReserveLargeMapOutput(mgr, (long)Integer.MAX_VALUE + 1);
          +  }
          +
          +  private void verifyReserveLargeMapOutput(MergeManagerImpl<Text, Text> mgr,
          +      long size) throws IOException {
          +    final TaskAttemptID mapId = TaskAttemptID.forName("attempt_0_1_m_1_1");
          +    final MapOutput<Text, Text> mapOutput = mgr.reserve(mapId, size, 1);
          +    assertEquals("Shuffled bytes: " + size,
          +        size <= Integer.MAX_VALUE ? "MEMORY" : "DISK",
          +        mapOutput.getDescription());
          +    mgr.unreserve(size);
             }
          

          In terms of the production code, testing this I noticed that

          mapreduce.task.reduce.MergeManagerImpl#canShuffleToMemory

          having '<' does not look right, and should be '<='. I suggest we fix it in this JIRA as well. Probably a good idea to inline this simple private one-liner.

          Show
          jira.shegalov Gera Shegalov added a comment - - edited As for test I was thinking of a smaller change to the existing test along the lines: diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java index 1c0d25b..5930139 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/task/reduce/TestMergeManager.java @@ -289,6 +289,19 @@ public void testLargeMemoryLimits() throws Exception { final long maxInMemReduce = mgr.getMaxInMemReduceLimit(); assertTrue( "Large in-memory reduce area unusable: " + maxInMemReduce, maxInMemReduce > Integer .MAX_VALUE); + + // verifyReserveLargeMapOutput(mgr, ( long ) Integer .MAX_VALUE); + verifyReserveLargeMapOutput(mgr, ( long ) Integer .MAX_VALUE + 1); + } + + private void verifyReserveLargeMapOutput(MergeManagerImpl<Text, Text> mgr, + long size) throws IOException { + final TaskAttemptID mapId = TaskAttemptID.forName( "attempt_0_1_m_1_1" ); + final MapOutput<Text, Text> mapOutput = mgr.reserve(mapId, size, 1); + assertEquals( "Shuffled bytes: " + size, + size <= Integer .MAX_VALUE ? "MEMORY" : "DISK" , + mapOutput.getDescription()); + mgr.unreserve(size); } In terms of the production code, testing this I noticed that mapreduce.task.reduce.MergeManagerImpl#canShuffleToMemory having '<' does not look right, and should be '<='. I suggest we fix it in this JIRA as well. Probably a good idea to inline this simple private one-liner.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 18m 50s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 33s trunk passed
          +1 compile 0m 25s trunk passed
          +1 checkstyle 0m 20s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 53s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 54s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          35m 3s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818660/mapreduce6724.005.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 3dd65b9b1252 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c2bcffb
          Default Java 1.8.0_91
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6621/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6621/console
          Powered by Apache Yetus 0.3.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 18m 50s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 33s trunk passed +1 compile 0m 25s trunk passed +1 checkstyle 0m 20s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 53s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 54s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 35m 3s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818660/mapreduce6724.005.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 3dd65b9b1252 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c2bcffb Default Java 1.8.0_91 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6621/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6621/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          LGTM. +1

          Show
          templedf Daniel Templeton added a comment - LGTM. +1
          Hide
          haibochen Haibo Chen added a comment -

          Yes, you are right. I have updated the patch again.

          Show
          haibochen Haibo Chen added a comment - Yes, you are right. I have updated the patch again.
          Hide
          templedf Daniel Templeton added a comment -

          Works for me. I'd not say that the value is irrelevant, though. It's just not part of that test. (If you set it to some terrible value, I guarantee it would be relevant.) Maybe just say it's not needed in this test.

          Show
          templedf Daniel Templeton added a comment - Works for me. I'd not say that the value is irrelevant, though. It's just not part of that test. (If you set it to some terrible value, I guarantee it would be relevant.) Maybe just say it's not needed in this test.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Daniel Templeton for your comments! The method createMergeManager is created per Gera Shegalov's comment and the variables are passed inline because their values are irrelevant to the tests. I have updated the patch with comments to note that.

          Show
          haibochen Haibo Chen added a comment - Thanks Daniel Templeton for your comments! The method createMergeManager is created per Gera Shegalov 's comment and the variables are passed inline because their values are irrelevant to the tests. I have updated the patch with comments to note that.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks for the patch, Haibo Chen! A couple comments:

              final long totalReduceMem = 8L * 1024 * 1024 * 1024;
              final float shuffleInputBuf = 1.0f;
              final float reduceInputBuf = 1.0f;
              final MergeManagerImpl<Text, Text> mgr = createMergeManager(
                  totalReduceMem, shuffleInputBuf, 0.95f, reduceInputBuf);
          

          It would be better to be consistent in the parameters. Everything is a variable except for the shuffle memory limit.

              final long totalReduceMem = 10000000000000000L;
              final float shuffleInputBuf = 1.0f;
              final float shuffleMemLimit =
                  singleShuffleLimitConfiged / (float) totalReduceMem;
              final MergeManagerImpl<Text, Text> mgr = createMergeManager(
                  totalReduceMem, shuffleInputBuf, shuffleMemLimit, 1.0f);
          

          Same comment here, except with reducer input buffer.

          Show
          templedf Daniel Templeton added a comment - Thanks for the patch, Haibo Chen ! A couple comments: final long totalReduceMem = 8L * 1024 * 1024 * 1024; final float shuffleInputBuf = 1.0f; final float reduceInputBuf = 1.0f; final MergeManagerImpl<Text, Text> mgr = createMergeManager( totalReduceMem, shuffleInputBuf, 0.95f, reduceInputBuf); It would be better to be consistent in the parameters. Everything is a variable except for the shuffle memory limit. final long totalReduceMem = 10000000000000000L; final float shuffleInputBuf = 1.0f; final float shuffleMemLimit = singleShuffleLimitConfiged / ( float ) totalReduceMem; final MergeManagerImpl<Text, Text> mgr = createMergeManager( totalReduceMem, shuffleInputBuf, shuffleMemLimit, 1.0f); Same comment here, except with reducer input buffer.
          Hide
          haibochen Haibo Chen added a comment -

          The unit test failure is known and not related to this patch.

          Show
          haibochen Haibo Chen added a comment - The unit test failure is known and not related to this patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 44s trunk passed
          +1 compile 0m 27s trunk passed
          +1 checkstyle 0m 19s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 56s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 26s the patch passed
          +1 javac 0m 26s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 30s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 2s The patch has no ill-formed XML file.
          +1 findbugs 1m 3s the patch passed
          +1 javadoc 0m 21s the patch passed
          -1 unit 2m 11s hadoop-mapreduce-client-core in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          17m 27s



          Reason Tests
          Failed junit tests hadoop.mapreduce.tools.TestCLI



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12816896/mapreduce6724.003.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux d1e02e36c91c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c04c5ec
          Default Java 1.8.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/console
          Powered by Apache Yetus 0.3.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 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 44s trunk passed +1 compile 0m 27s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 35s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 56s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 30s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 21s the patch passed -1 unit 2m 11s hadoop-mapreduce-client-core in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 17m 27s Reason Tests Failed junit tests hadoop.mapreduce.tools.TestCLI Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12816896/mapreduce6724.003.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux d1e02e36c91c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c04c5ec Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6603/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks a lot for your reviews, Gera Shegalov! And my apologies for being late to update the patch. I have updated the patch to accommodate your comments except for one thing. Instead of creating a method that just takes targetSingleShuffleLimit as its parameter, I have four parameters for the new private method, since the setup of testLargeMemoryLimits involves REDUCE_MEMORY_TOTAL_BYTES, SHUFFLE_INPUT_BUFFER_PERCENT and REDUCE_INPUT_BUFFER_PERCENT whereas the new test method involves REDUCE_MEMORY_TOTAL_BYTES, SHUFFLE_INPUT_BUFFER_PERCENT and SHUFFLE_MEMORY_LIMIT_PERCENT (testLargeMemoryLimits does not care about the value of targetSingleShuffleLimit). Thanks again for your review and please let me know any more comments you have.

          Show
          haibochen Haibo Chen added a comment - Thanks a lot for your reviews, Gera Shegalov ! And my apologies for being late to update the patch. I have updated the patch to accommodate your comments except for one thing. Instead of creating a method that just takes targetSingleShuffleLimit as its parameter, I have four parameters for the new private method, since the setup of testLargeMemoryLimits involves REDUCE_MEMORY_TOTAL_BYTES, SHUFFLE_INPUT_BUFFER_PERCENT and REDUCE_INPUT_BUFFER_PERCENT whereas the new test method involves REDUCE_MEMORY_TOTAL_BYTES, SHUFFLE_INPUT_BUFFER_PERCENT and SHUFFLE_MEMORY_LIMIT_PERCENT (testLargeMemoryLimits does not care about the value of targetSingleShuffleLimit). Thanks again for your review and please let me know any more comments you have.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Thanks for the patch Haibo Chen. a couple of suggestions after skimming the patch.

          • It would be helpful to have a diagnostic LOG.info message that the single shuffle limit is less than would be per mapreduce.reduce.shuffle.memory.limit.percent config.
          • we can have a much smaller patch.
            • instead of doing refactoring to make maxSingleShuffleLimit visible simply observe the result of mapOutput.getDescription similar to testZeroShuffleMemoryLimitPercent
            • similar tests did not have to introduce extra mock classes, and we can do without it in this patch as well
            • the easiest seems to factor out some common logic from testLargeMemoryLimits into a private method that can accept targetSingleShuffleLimit as a parameter. Then you can do:
              conf.setFloat(MRJobConfig.SHUFFLE_MEMORY_LIMIT_PERCENT, ((float)targetSingleShuffleLimit / conf.getLong(MRJobConfig.REDUCE_MEMORY_TOTAL_BYTES)));
              

              We can assert DISK for targetSingleShuffleLimit > Integer.MAX_VALUE and MEMORY otherwise.

          Show
          jira.shegalov Gera Shegalov added a comment - Thanks for the patch Haibo Chen . a couple of suggestions after skimming the patch. It would be helpful to have a diagnostic LOG.info message that the single shuffle limit is less than would be per mapreduce.reduce.shuffle.memory.limit.percent config. we can have a much smaller patch. instead of doing refactoring to make maxSingleShuffleLimit visible simply observe the result of mapOutput.getDescription similar to testZeroShuffleMemoryLimitPercent similar tests did not have to introduce extra mock classes, and we can do without it in this patch as well the easiest seems to factor out some common logic from testLargeMemoryLimits into a private method that can accept targetSingleShuffleLimit as a parameter. Then you can do: conf.setFloat(MRJobConfig.SHUFFLE_MEMORY_LIMIT_PERCENT, (( float )targetSingleShuffleLimit / conf.getLong(MRJobConfig.REDUCE_MEMORY_TOTAL_BYTES))); We can assert DISK for targetSingleShuffleLimit > Integer.MAX_VALUE and MEMORY otherwise.
          Hide
          haibochen Haibo Chen added a comment -

          The test failure is reported at MAPREDUCE-6625. Does not seem to be related to this patch.

          Show
          haibochen Haibo Chen added a comment - The test failure is reported at MAPREDUCE-6625 . Does not seem to be related to this patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 26s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 29s trunk passed
          +1 compile 0m 24s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 0m 47s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 22s the patch passed
          +1 javac 0m 22s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 27s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 57s the patch passed
          +1 javadoc 0m 21s the patch passed
          -1 unit 2m 4s hadoop-mapreduce-client-core in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          15m 24s



          Reason Tests
          Failed junit tests hadoop.mapreduce.tools.TestCLI



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:85209cc
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812906/mapreduce6724.002.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 6f0a5b9112fc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / e98c0c7
          Default Java 1.8.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/console
          Powered by Apache Yetus 0.3.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 26s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 29s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 0m 47s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 22s the patch passed +1 javac 0m 22s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 27s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 57s the patch passed +1 javadoc 0m 21s the patch passed -1 unit 2m 4s hadoop-mapreduce-client-core in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 15m 24s Reason Tests Failed junit tests hadoop.mapreduce.tools.TestCLI Subsystem Report/Notes Docker Image:yetus/hadoop:85209cc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812906/mapreduce6724.002.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 6f0a5b9112fc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e98c0c7 Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6581/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Patch updated to fix checkstyle issues.

          Show
          haibochen Haibo Chen added a comment - Patch updated to fix checkstyle issues.
          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 1 new or modified test files.
          +1 mvninstall 6m 18s trunk passed
          +1 compile 0m 23s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 46s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 14s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core: The patch generated 8 new + 101 unchanged - 0 fixed = 109 total (was 101)
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 52s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 1m 58s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          14m 38s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812715/mapreduce6724.001.patch
          JIRA Issue MAPREDUCE-6724
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 3ae25b2aea66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6ab5aa1
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/console
          Powered by Apache Yetus 0.3.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 1 new or modified test files. +1 mvninstall 6m 18s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 46s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 14s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core: The patch generated 8 new + 101 unchanged - 0 fixed = 109 total (was 101) +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 1m 58s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 14m 38s Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812715/mapreduce6724.001.patch JIRA Issue MAPREDUCE-6724 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 3ae25b2aea66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6ab5aa1 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6578/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Gera Shegalov a lot for reviewing the approach proposed. Uploading a patch that implements it.
          1) The patch caps maxSingleShuffleLimit at Integer.MAX_VALUE, so that all shuffle with output bigger than Integer.MAX_VALUE will be done on disk, avoiding the unsafe conversion
          2) The reset of the patch is just refactoring MergeManagerImpl so that a no-merge MergeManager can be created in the new test.

          Show
          haibochen Haibo Chen added a comment - Thanks Gera Shegalov a lot for reviewing the approach proposed. Uploading a patch that implements it. 1) The patch caps maxSingleShuffleLimit at Integer.MAX_VALUE, so that all shuffle with output bigger than Integer.MAX_VALUE will be done on disk, avoiding the unsafe conversion 2) The reset of the patch is just refactoring MergeManagerImpl so that a no-merge MergeManager can be created in the new test.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Good catch, Haibo Chen, and maxSingleShuffleLimit sounds like a good approach.

          Show
          jira.shegalov Gera Shegalov added a comment - Good catch, Haibo Chen , and maxSingleShuffleLimit sounds like a good approach.
          Hide
          haibochen Haibo Chen added a comment -

          The implementation of InMemoryMapOutput uses a byte array, which only support a maximum size of Integer.MAX_VALUE. The conversion of requested size from long to int seems unavoidable if we want to stick with the byte array implementation.
          We could cap the value of maxSingleShuffleLimit at Integer.MAX_VALUE, so shuffle is done on disk if the requested size is bigger than Integer.MAX_VALUE.

          Show
          haibochen Haibo Chen added a comment - The implementation of InMemoryMapOutput uses a byte array, which only support a maximum size of Integer.MAX_VALUE. The conversion of requested size from long to int seems unavoidable if we want to stick with the byte array implementation. We could cap the value of maxSingleShuffleLimit at Integer.MAX_VALUE, so shuffle is done on disk if the requested size is bigger than Integer.MAX_VALUE.

            People

            • Assignee:
              haibochen Haibo Chen
              Reporter:
              haibochen Haibo Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development