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

Derive heap size or mapreduce.*.memory.mb automatically

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: mr-am, task
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Incompatible change

      Description

      Currently users have to set 2 memory-related configs per Job / per task type. One first chooses some container size map reduce.*.memory.mb and then a corresponding maximum Java heap size Xmx < map reduce.*.memory.mb. This makes sure that the JVM's C-heap (native memory + Java heap) does not exceed this mapreduce.*.memory.mb. If one forgets to tune Xmx, MR-AM might be

      • allocating big containers whereas the JVM will only use the default -Xmx200m.
      • allocating small containers that will OOM because Xmx is too high.

      With this JIRA, we propose to set Xmx automatically based on an empirical ratio that can be adjusted. Xmx is not changed automatically if provided by the user.

      1. mr-5785-9.patch
        23 kB
        Karthik Kambatla
      2. mr-5785-8.patch
        23 kB
        Karthik Kambatla
      3. mr-5785-7.patch
        23 kB
        Karthik Kambatla
      4. mr-5785-6.patch
        24 kB
        Karthik Kambatla
      5. mr-5785-5.patch
        24 kB
        Karthik Kambatla
      6. mr-5785-4.patch
        25 kB
        Karthik Kambatla
      7. MAPREDUCE-5785.v03.patch
        25 kB
        Gera Shegalov
      8. MAPREDUCE-5785.v02.patch
        26 kB
        Gera Shegalov
      9. MAPREDUCE-5785.v01.patch
        10 kB
        Gera Shegalov

        Issue Links

          Activity

          Hide
          Allen Wittenauer added a comment -

          Someone should write a release note for this change, since it is a fairly significant change in behavior....

          Show
          Allen Wittenauer added a comment - Someone should write a release note for this change, since it is a fairly significant change in behavior....
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2059 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2059/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          • hadoop-mapreduce-project/CHANGES.txt
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2059 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2059/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java hadoop-mapreduce-project/CHANGES.txt
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #109 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/109/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #109 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/109/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-mapreduce-project/CHANGES.txt hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #2040 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2040/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2040 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2040/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-mapreduce-project/CHANGES.txt hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #99 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/99/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          • hadoop-mapreduce-project/CHANGES.txt
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #99 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/99/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java hadoop-mapreduce-project/CHANGES.txt
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #108 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/108/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          • hadoop-mapreduce-project/CHANGES.txt
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #108 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/108/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java hadoop-mapreduce-project/CHANGES.txt
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #842 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/842/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #842 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/842/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-mapreduce-project/CHANGES.txt hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7136 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7136/)
          MAPREDUCE-6234. TestHighRamJob fails due to the change in MAPREDUCE-5785. (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0)

          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7136 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7136/ ) MAPREDUCE-6234 . TestHighRamJob fails due to the change in MAPREDUCE-5785 . (Masatake Iwasaki via kasha) (kasha: rev 409113d8f97fcfdb96cb028dbb6a20c9a1df81b0) hadoop-mapreduce-project/CHANGES.txt hadoop-tools/hadoop-gridmix/src/test/java/org/apache/hadoop/mapred/gridmix/TestHighRamJob.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2032 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2032/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2032 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2032/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #82 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/82/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #82 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/82/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #78 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/78/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #78 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/78/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2013 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2013/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2013 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2013/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #815 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/815/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #815 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/815/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #81 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/81/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #81 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/81/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #6910 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6910/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #6910 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6910/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via gera) (gera: rev a003f71cacd35834a1abbc2ffb5446a1166caf73) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          Hide
          Gera Shegalov added a comment -

          Committed to trunk. Thanks Karthik Kambatla for collaborating on this patch!

          Show
          Gera Shegalov added a comment - Committed to trunk. Thanks Karthik Kambatla for collaborating on this patch!
          Hide
          Gera Shegalov added a comment -

          findbugs warnings are unrelated. +1 for the v9 patch. Thanks for accommodating my comments, Karthik. I'll commit it tomorrow evening if there are no objections.

          Show
          Gera Shegalov added a comment - findbugs warnings are unrelated. +1 for the v9 patch. Thanks for accommodating my comments, Karthik. I'll commit it tomorrow evening if there are no objections.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12693467/mr-5785-9.patch
          against trunk revision fc20129.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

          -1 findbugs. The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12693467/mr-5785-9.patch against trunk revision fc20129. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5113//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Updated patch to address Gera's latest comments.

          Show
          Karthik Kambatla added a comment - Updated patch to address Gera's latest comments.
          Hide
          Gera Shegalov added a comment -

          Hi Karthik Kambatla, thanks for updating the patch.

          nit: v8 patch introduced a line break in the class declaration

          public class
              TestMapReduceChildJVM {
          

          The config convention/implementation equates an empty value in xml to the unset value. So I think your v7 was fine in this respect.

          If it looks too hacky to you, we can set memory.mb keys to -1, the default you used in v8 or maybe more natural 0. Then we can say memory.mb <= 0 implies not set.

          I prefer fail-fast validation, so I am not in favor of catching NumberFormatException. At least not in this patch. Otherwise, it's not clear why are we not doing this for all numeric parameters.

          Show
          Gera Shegalov added a comment - Hi Karthik Kambatla , thanks for updating the patch. nit: v8 patch introduced a line break in the class declaration public class TestMapReduceChildJVM { The config convention/implementation equates an empty value in xml to the unset value. So I think your v7 was fine in this respect. If it looks too hacky to you, we can set memory.mb keys to -1 , the default you used in v8 or maybe more natural 0 . Then we can say memory.mb <= 0 implies not set . I prefer fail-fast validation, so I am not in favor of catching NumberFormatException. At least not in this patch. Otherwise, it's not clear why are we not doing this for all numeric parameters.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12693106/mr-5785-8.patch
          against trunk revision 4a5c3a4.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

          -1 findbugs. The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12693106/mr-5785-8.patch against trunk revision 4a5c3a4. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5107//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Thanks for your thorough review, Gera Shegalov. The updated patch (v8) addresses your first 3 comments.

          Regarding the mapred-default suggestions for memory.mb, I don't think it is an issue of consistency. Unlike other configs, an empty string is not really a valid value. The latest patch has changes to gracefully handle invalid values - those that lead to NumberFormatExceptions. If people insist, I am okay with changing the default value to an empty string.

          Show
          Karthik Kambatla added a comment - Thanks for your thorough review, Gera Shegalov . The updated patch (v8) addresses your first 3 comments. Regarding the mapred-default suggestions for memory.mb, I don't think it is an issue of consistency. Unlike other configs, an empty string is not really a valid value. The latest patch has changes to gracefully handle invalid values - those that lead to NumberFormatExceptions. If people insist, I am okay with changing the default value to an empty string.
          Hide
          Gera Shegalov added a comment -

          Karthik Kambatla, thanks for working on this patch. It's look good. I've got a few suggestions:

          1. TaskAttemptImpl#getMemoryRequired:
          It can be written as a one-liner as follows:

            private int getMemoryRequired(JobConf conf, TaskType taskType) {
              return conf.getMemoryRequired(TypeConverter.fromYarn(taskType));
            }
          

          2. TestMapReduceChildJVM#testAutoHeapSize
          We don't need to create 2 objects via new JobConf(new Configuration()), why not simply have

             JobConf conf = new JobConf();
          

          3. JobConf#getConfiguredTaskJavaOpts
          Instead of doing String#equals("") let us use String#isEmpty
          Checking user/adminClasspath for null is redundant because they are actually known to be not null. I also wonder whether preventing a single extra space in the command line is worth 7 lines of code

          4. mapred-default.xml
          Unsetting default values is inconsistent within the patch and outside the patch. For mapred.child.java.opts we use an empty <value> like for many other parameters, but for mapreduce.*.memory.mb the value element is removed by commenting it out. I think it should be done the same way as mapred.child.java.opts , and the comment explaining the defaults is enough.

          Show
          Gera Shegalov added a comment - Karthik Kambatla , thanks for working on this patch. It's look good. I've got a few suggestions: 1. TaskAttemptImpl#getMemoryRequired : It can be written as a one-liner as follows: private int getMemoryRequired(JobConf conf, TaskType taskType) { return conf.getMemoryRequired(TypeConverter.fromYarn(taskType)); } 2. TestMapReduceChildJVM#testAutoHeapSize We don't need to create 2 objects via new JobConf(new Configuration()) , why not simply have JobConf conf = new JobConf(); 3. JobConf#getConfiguredTaskJavaOpts Instead of doing String#equals("") let us use String#isEmpty Checking user/adminClasspath for null is redundant because they are actually known to be not null. I also wonder whether preventing a single extra space in the command line is worth 7 lines of code 4. mapred-default.xml Unsetting default values is inconsistent within the patch and outside the patch. For mapred.child.java.opts we use an empty <value> like for many other parameters, but for mapreduce.*.memory.mb the value element is removed by commenting it out. I think it should be done the same way as mapred.child.java.opts , and the comment explaining the defaults is enough.
          Hide
          Robert Kanter added a comment -

          Looks good to me. +1 pending Gera Shegalov's review.
          Karthik Kambatla, because this is an incompatible change, don't forget to add something to the "Release Note" field in the JIRA.

          Show
          Robert Kanter added a comment - Looks good to me. +1 pending Gera Shegalov 's review. Karthik Kambatla , because this is an incompatible change, don't forget to add something to the "Release Note" field in the JIRA.
          Hide
          Gera Shegalov added a comment -

          Hi Karthik, thanks for updating the patch. I'll take a look early next week.

          Show
          Gera Shegalov added a comment - Hi Karthik, thanks for updating the patch. I'll take a look early next week.
          Hide
          Karthik Kambatla added a comment -

          Assigned to myself for better tracking, will assign back to Gera at commit time.

          The findbugs warnings seem unrelated.

          Show
          Karthik Kambatla added a comment - Assigned to myself for better tracking, will assign back to Gera at commit time. The findbugs warnings seem unrelated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12690903/mr-5785-7.patch
          against trunk revision 708b1aa.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

          -1 findbugs. The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12690903/mr-5785-7.patch against trunk revision 708b1aa. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 13 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//artifact/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5101//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Here is an updated patch that fixes the earlier issues. As Gera suggested, I set the default value to be an empty string and left the tests as is.

          Gera Shegalov - can you review the latest patch when you get a chance?

          Show
          Karthik Kambatla added a comment - Here is an updated patch that fixes the earlier issues. As Gera suggested, I set the default value to be an empty string and left the tests as is. Gera Shegalov - can you review the latest patch when you get a chance?
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/17/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/17/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1969 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1969/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1969 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1969/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/17/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/17/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #1945 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1945/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1945 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1945/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #755 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/755/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #755 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/755/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/17/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #17 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/17/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #6607 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6607/)
          Revert "MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #6607 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6607/ ) Revert " MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha)" (kasha: rev a655973e781caf662b360c96e0fa3f5a873cf676) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          Hide
          Karthik Kambatla added a comment -

          Reverted. Let me fix the bug and post another patch.

          Show
          Karthik Kambatla added a comment - Reverted. Let me fix the bug and post another patch.
          Hide
          Karthik Kambatla added a comment -

          Actually, this is a bug with the patch itself. It might be best to revert it for now until we fix the issue. Reverting it.

          Show
          Karthik Kambatla added a comment - Actually, this is a bug with the patch itself. It might be best to revert it for now until we fix the issue. Reverting it.
          Hide
          Jian He added a comment -

          After this patch, job somehow fails due to not able to launch task container Error: Could not find or load main class null. (might be my own setup problem)

          Show
          Jian He added a comment - After this patch, job somehow fails due to not able to launch task container Error: Could not find or load main class null . (might be my own setup problem)
          Hide
          Karthik Kambatla added a comment -

          In my patch I was setting DEFAULT_MAPRED_TASK_JAVA_OPTS to "" to make sure that we don't get null. In the latest patch it actually happens and is not caught because the tests are weakened by checking only for containment of command fragments instead of checking the whole command.

          Thanks for the input, Gera. You are right. I didn't realize I was weakening the test when I refactored it. Let me reopen the JIRA and revert the refactoring in an addendum patch.

          Show
          Karthik Kambatla added a comment - In my patch I was setting DEFAULT_MAPRED_TASK_JAVA_OPTS to "" to make sure that we don't get null. In the latest patch it actually happens and is not caught because the tests are weakened by checking only for containment of command fragments instead of checking the whole command. Thanks for the input, Gera. You are right. I didn't realize I was weakening the test when I refactored it. Let me reopen the JIRA and revert the refactoring in an addendum patch.
          Hide
          Allen Wittenauer added a comment -

          I think we should commit this to branch-2 as well:

          Nope, no way. As Hitesh Shah points out, this is an incompatible change that will impact a portion of the of the user base. The list of potential impacts are all pretty nasty for certain use cases. (hint: some people run Hadoop on embedded and small memory devices...)

          Show
          Allen Wittenauer added a comment - I think we should commit this to branch-2 as well: Nope, no way. As Hitesh Shah points out, this is an incompatible change that will impact a portion of the of the user base. The list of potential impacts are all pretty nasty for certain use cases. (hint: some people run Hadoop on embedded and small memory devices...)
          Hide
          Hitesh Shah added a comment -

          I think we should commit this to branch-2 as well

          This change is incompatible especially as it modifies mapred-default.xml. Not sure why it would be committed to branch-2.

          Show
          Hitesh Shah added a comment - I think we should commit this to branch-2 as well This change is incompatible especially as it modifies mapred-default.xml. Not sure why it would be committed to branch-2.
          Hide
          Gera Shegalov added a comment -

          Hi Karthik Kambatla,

          Sorry, I have not had a chance to look at your changes earlier.
          In my patch I was setting DEFAULT_MAPRED_TASK_JAVA_OPTS to "" to make sure that we don't get null. In the latest patch it actually happens and is not caught because the tests are weakened by checking only for containment of command fragments instead of checking the whole command.

          Show
          Gera Shegalov added a comment - Hi Karthik Kambatla , Sorry, I have not had a chance to look at your changes earlier. In my patch I was setting DEFAULT_MAPRED_TASK_JAVA_OPTS to "" to make sure that we don't get null. In the latest patch it actually happens and is not caught because the tests are weakened by checking only for containment of command fragments instead of checking the whole command.
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1965 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1965/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1965 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1965/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/13/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/13/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/13/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/13/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1941 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1941/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/CHANGES.txt
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1941 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1941/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/CHANGES.txt
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #751 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/751/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #751 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/751/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/13/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #13 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/13/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Karthik Kambatla added a comment -

          Thanks for your review, Sandy. I just committed this to trunk.

          I think we should commit this to branch-2 as well:

          1. The patch doesn't affect users who set java.opts
          2. For users that set java.opts but without -Xmx, this would limit their heap size and could lead to task failures. I am not too concerned about this, as we expect them to set java.opts and anyway the container would have been killed by YARN if physical memory constraints are enforced.
          3. For users that don't set java.opts, this increases the JVM size from 200 MB to 820 MB. This should be okay for most jobs; streaming tasks might end up needing more memory because of their java process taking their total usage over the container size. Even here, this would likely happen only for those tasks relying on aggressive GC to keep the java.opts under 200 MB.

          All in all, I believe the likely cases affect by this is small enough that it should be okay to include this in a release, provided we release-note carefully.

          Allen Wittenauer, Jason Lowe, Sandy Ryza - thoughts?

          Show
          Karthik Kambatla added a comment - Thanks for your review, Sandy. I just committed this to trunk. I think we should commit this to branch-2 as well: The patch doesn't affect users who set java.opts For users that set java.opts but without -Xmx, this would limit their heap size and could lead to task failures. I am not too concerned about this, as we expect them to set java.opts and anyway the container would have been killed by YARN if physical memory constraints are enforced. For users that don't set java.opts, this increases the JVM size from 200 MB to 820 MB. This should be okay for most jobs; streaming tasks might end up needing more memory because of their java process taking their total usage over the container size. Even here, this would likely happen only for those tasks relying on aggressive GC to keep the java.opts under 200 MB. All in all, I believe the likely cases affect by this is small enough that it should be okay to include this in a release, provided we release-note carefully. Allen Wittenauer , Jason Lowe , Sandy Ryza - thoughts?
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #6591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6591/)
          MAPREDUCE-5785. Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30)

          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java
          • hadoop-mapreduce-project/CHANGES.txt
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java
          • hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #6591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6591/ ) MAPREDUCE-5785 . Derive heap size or mapreduce.*.memory.mb automatically. (Gera Shegalov and Karthik Kambatla via kasha) (kasha: rev a4df9eed059977374c8e889cb85d79e8e514ad30) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java hadoop-mapreduce-project/CHANGES.txt hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TaskAttemptImpl.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/Task.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/MapReduceChildJVM.java hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Sandy Ryza added a comment -

          +1

          Show
          Sandy Ryza added a comment - +1
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12682982/mr-5785-6.patch
          against trunk revision 1e9a3f4.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12682982/mr-5785-6.patch against trunk revision 1e9a3f4. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5042//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5042//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Thanks Sandy. Missed that - removed that and other cases where we use final for method-local variables.

          Show
          Karthik Kambatla added a comment - Thanks Sandy. Missed that - removed that and other cases where we use final for method-local variables.
          Hide
          Sandy Ryza added a comment -

          This looks almost there. Leaving the indentation seems fine to me.

          JobConf is already kind of a god class. I think that the more we can avoid further cluttering it by moving code closer to its access points, the better.

          +    final JobConf conf = new JobConf(new Configuration());
          

          The patch uses final in a lot of places that MR code conventionally does not. Even if this is better practice, I don't think now is the time to start.

          Show
          Sandy Ryza added a comment - This looks almost there. Leaving the indentation seems fine to me. JobConf is already kind of a god class. I think that the more we can avoid further cluttering it by moving code closer to its access points, the better. + final JobConf conf = new JobConf( new Configuration()); The patch uses final in a lot of places that MR code conventionally does not. Even if this is better practice, I don't think now is the time to start.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12682912/mr-5785-5.patch
          against trunk revision c298a9a.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12682912/mr-5785-5.patch against trunk revision c298a9a. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5041//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5041//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment - - edited

          Thanks for the review, Sandy. Addressed most of your comments.

          It looks like this value will be overwritten in all cases.

          Actually no. We use an if - else if without an else.

          This is a little weird. Can we assign heapSize outside of the condition?

          I changed this in the updated patch, but this was to avoid parsing the heapSize unnecessarily or complicating the code with more if-else's.

          Indentation here is inconsistent with other places.

          The file has different kinds of indentation. I would like to leave this as is, if you are not particular about it.

          Any reason to have getTaskJavaOpts in JobConf instead of MapReduceChildJVM?

          MapReduceChildJVM needs the javaopts and TaskAttemptImpl needs the memory. We could keep those two methods closer to the access points, but I felt the two methods should be closer together than to the access points.

          Show
          Karthik Kambatla added a comment - - edited Thanks for the review, Sandy. Addressed most of your comments. It looks like this value will be overwritten in all cases. Actually no. We use an if - else if without an else. This is a little weird. Can we assign heapSize outside of the condition? I changed this in the updated patch, but this was to avoid parsing the heapSize unnecessarily or complicating the code with more if-else's. Indentation here is inconsistent with other places. The file has different kinds of indentation. I would like to leave this as is, if you are not particular about it. Any reason to have getTaskJavaOpts in JobConf instead of MapReduceChildJVM? MapReduceChildJVM needs the javaopts and TaskAttemptImpl needs the memory. We could keep those two methods closer to the access points, but I felt the two methods should be closer together than to the access points.
          Hide
          Sandy Ryza added a comment -

          Hi Karthik. Took a look at the patch. Had some comments - mostly stylistic.

          -    return adminClasspath + " " + userClasspath;
          +      return jobConf.getTaskJavaOpts(isMapTask ? TaskType.MAP : TaskType.REDUCE);
          

          Wrong indentation?

          +  public String getTaskJavaOpts(TaskType taskType) {
          +
          +    String javaOpts = getConfiguredTaskJavaOpts(taskType);
          

          Unnecessary blank line.

          +      LOG.info("Task java.opts does not specify max heap size, setting using"
          +          + " mapreduce.*.memory.mb * "
          +          + MRJobConfig.HEAP_MEMORY_MB_RATIO);
          

          Can we condense this and the log further down into a single message?

          +        if (LOG.isWarnEnabled()) {
          

          Why use isWarnEnabled when we don't use isInfoEnabled?

          +      final int taskContainerMb = getMemoryRequired(taskType);
          

          Any reason this should be final? Convention is usually not to declare local variables final unless they need to be (like referenced by an anonymous class).

          +      int taskHeapSize =(int)Math.ceil(taskContainerMb * heapRatio);
          

          Should have a space after the "=".

          +  public static int parseMaximumHeapSizeMB(String javaOpts) {
          

          Can this (and others) be marked Private?

          +    int memory = 1024;
          

          It looks like this value will be overwritten in all cases.

          +          (heapSize = parseMaximumHeapSizeMB(
          +              getConfiguredTaskJavaOpts(taskType))) > 0) {
          

          This is a little weird. Can we assign heapSize outside of the condition?

          +        memory =
          +            getInt(MRJobConfig.REDUCE_MEMORY_MB,
          +                MRJobConfig.DEFAULT_REDUCE_MEMORY_MB);
          

          This can be on 2 lines.

          +  If -Xmx is not set, it is inferred from mapreduc.{map|reduce}.memory.mb and
          

          Missing an "e" at the end of mapreduc.

          +  <description>The ratio container between heap-size and container-size
          +    If no -Xmx specified, it's calculated from the container memory
          +    requirement: mapreduce.*.memory.mb * mapreduce.heap.memory-mb.ratio.
          +    If -Xmx is specified but not mapreduce.*.memory.mb, it's calculated as
          +    heapSize / mapreduce.heap.memory-mb.ratio.
          

          Need a period after container size. "*" meaning both multiplication and either map or reduce is a little confusing here. It might be better to spell out

          {map|reduce}

          inside the config properties, which would also be consistent with how they're references above. Also, other descriptions tend to use "it is" instead of "it's".

             <description>The amount of memory to request from the scheduler for each
          -  reduce task.
          +    reduce task. If this is not specified, it is inferred from
          

          Indentation here is inconsistent with other places.

          Any reason to have getTaskJavaOpts in JobConf instead of MapReduceChildJVM?

          Show
          Sandy Ryza added a comment - Hi Karthik. Took a look at the patch. Had some comments - mostly stylistic. - return adminClasspath + " " + userClasspath; + return jobConf.getTaskJavaOpts(isMapTask ? TaskType.MAP : TaskType.REDUCE); Wrong indentation? + public String getTaskJavaOpts(TaskType taskType) { + + String javaOpts = getConfiguredTaskJavaOpts(taskType); Unnecessary blank line. + LOG.info( "Task java.opts does not specify max heap size, setting using" + + " mapreduce.*.memory.mb * " + + MRJobConfig.HEAP_MEMORY_MB_RATIO); Can we condense this and the log further down into a single message? + if (LOG.isWarnEnabled()) { Why use isWarnEnabled when we don't use isInfoEnabled? + final int taskContainerMb = getMemoryRequired(taskType); Any reason this should be final? Convention is usually not to declare local variables final unless they need to be (like referenced by an anonymous class). + int taskHeapSize =( int ) Math .ceil(taskContainerMb * heapRatio); Should have a space after the "=". + public static int parseMaximumHeapSizeMB( String javaOpts) { Can this (and others) be marked Private? + int memory = 1024; It looks like this value will be overwritten in all cases. + (heapSize = parseMaximumHeapSizeMB( + getConfiguredTaskJavaOpts(taskType))) > 0) { This is a little weird. Can we assign heapSize outside of the condition? + memory = + getInt(MRJobConfig.REDUCE_MEMORY_MB, + MRJobConfig.DEFAULT_REDUCE_MEMORY_MB); This can be on 2 lines. + If -Xmx is not set, it is inferred from mapreduc.{map|reduce}.memory.mb and Missing an "e" at the end of mapreduc. + <description>The ratio container between heap-size and container-size + If no -Xmx specified, it's calculated from the container memory + requirement: mapreduce.*.memory.mb * mapreduce.heap.memory-mb.ratio. + If -Xmx is specified but not mapreduce.*.memory.mb, it's calculated as + heapSize / mapreduce.heap.memory-mb.ratio. Need a period after container size. "*" meaning both multiplication and either map or reduce is a little confusing here. It might be better to spell out {map|reduce} inside the config properties, which would also be consistent with how they're references above. Also, other descriptions tend to use "it is" instead of "it's". <description>The amount of memory to request from the scheduler for each - reduce task. + reduce task. If this is not specified, it is inferred from Indentation here is inconsistent with other places. Any reason to have getTaskJavaOpts in JobConf instead of MapReduceChildJVM?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12682773/mr-5785-4.patch
          against trunk revision eb4045e.

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12682773/mr-5785-4.patch against trunk revision eb4045e. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5040//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5040//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Gera Shegalov, Jason Lowe - can you take a look at the patch when you get a chance?

          Show
          Karthik Kambatla added a comment - Gera Shegalov , Jason Lowe - can you take a look at the patch when you get a chance?
          Hide
          Karthik Kambatla added a comment -

          Uploading a patch that does the following:

          1. If -Xmx is not set in the conf, it calculates this from the memory.mb value.
          2. If memory.mb is not set in the conf, it calculates this from the -Xmx value.
          3. If neither are set, it uses the default memory.mb value and calculates -Xmx from this.
          4. Updates a bunch of descriptions to mapred-default.xml to capture this clearly. Let me know if they are not clear enough.
          5. A little bit of test cleanup.

          The patch reverts changes to io.sort.mb from earlier versions to be addressed in a follow-up JIRA.

          To parse the -Xmx value from java-opts, the patch "borrows" Jonathan Eagles patch from TEZ-1508. Jonathan Eagles - hope you are fine with that.

          Show
          Karthik Kambatla added a comment - Uploading a patch that does the following: If -Xmx is not set in the conf, it calculates this from the memory.mb value. If memory.mb is not set in the conf, it calculates this from the -Xmx value. If neither are set, it uses the default memory.mb value and calculates -Xmx from this. Updates a bunch of descriptions to mapred-default.xml to capture this clearly. Let me know if they are not clear enough. A little bit of test cleanup. The patch reverts changes to io.sort.mb from earlier versions to be addressed in a follow-up JIRA. To parse the -Xmx value from java-opts, the patch "borrows" Jonathan Eagles patch from TEZ-1508 . Jonathan Eagles - hope you are fine with that.
          Hide
          Karthik Kambatla added a comment -

          Are you going to check conf.getPropertySources for whether it was modifed.

          I was considering just removing the default value from mapred-default, and use the one from code.

          If one of heap-size or memory.mb is specified, I believe we should automatically pick the other one. If neither are specified, we should go off memory-mb default.

          Show
          Karthik Kambatla added a comment - Are you going to check conf.getPropertySources for whether it was modifed. I was considering just removing the default value from mapred-default, and use the one from code. If one of heap-size or memory.mb is specified, I believe we should automatically pick the other one. If neither are specified, we should go off memory-mb default.
          Hide
          Gera Shegalov added a comment -

          Karthik Kambatla, I agree that for MRv1->MRv2 transition deriving the container size from Xmx is a better fit. Feel free to take over this JIRA.

          A challenge here might be is that there is a (default) value for *.memory.mb. How do you know it's ok to modify it? Are you going to check conf.getPropertySources for whether it was modifed. Or do you want to introduce another boolean switch that disables overrides?

          Show
          Gera Shegalov added a comment - Karthik Kambatla , I agree that for MRv1->MRv2 transition deriving the container size from Xmx is a better fit. Feel free to take over this JIRA. A challenge here might be is that there is a (default) value for *.memory.mb. How do you know it's ok to modify it? Are you going to check conf.getPropertySources for whether it was modifed. Or do you want to introduce another boolean switch that disables overrides?
          Hide
          Allen Wittenauer added a comment - - edited

          Technically this may need to be marked as an incompatible change

          I agree: this will likely break all kinds of user jobs. I don't even want to consider what happens in the streaming and pipes use cases....

          Show
          Allen Wittenauer added a comment - - edited Technically this may need to be marked as an incompatible change I agree: this will likely break all kinds of user jobs. I don't even want to consider what happens in the streaming and pipes use cases....
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12644318/MAPREDUCE-5785.v03.patch
          against trunk revision 48d62fa.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5025//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12644318/MAPREDUCE-5785.v03.patch against trunk revision 48d62fa. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5025//console This message is automatically generated.
          Hide
          Karthik Kambatla added a comment -

          Sorry for the dropping the ball on this. Few comments:

          1. For those moving from MR1 to MR2, specifying Xmx might be easier. In those cases, it would be nice to estimate the memory.mb values. I am open to doing this in another JIRA, and would be happy to work on it and post a patch.
          2. Multiplying container-size by 0.8 to arrive at the heapsize does read better. We will have to divide by 0.8 to do it in the other direction.
          3. In the patch itself, deprecating mapred.child.java.opts is fine but I would refer to .map.java.opts and .reduce.java.opts instead of the container size values. Those properties instead could point to the memory.mb properties.

          I understand it has been a while since we last looked at this. Gera Shegalov - will you be able to address these comments? I can help with addressing the comments if you are otherwise caught up.

          Show
          Karthik Kambatla added a comment - Sorry for the dropping the ball on this. Few comments: For those moving from MR1 to MR2, specifying Xmx might be easier. In those cases, it would be nice to estimate the memory.mb values. I am open to doing this in another JIRA, and would be happy to work on it and post a patch. Multiplying container-size by 0.8 to arrive at the heapsize does read better. We will have to divide by 0.8 to do it in the other direction. In the patch itself, deprecating mapred.child.java.opts is fine but I would refer to .map.java.opts and .reduce.java.opts instead of the container size values. Those properties instead could point to the memory.mb properties. I understand it has been a while since we last looked at this. Gera Shegalov - will you be able to address these comments? I can help with addressing the comments if you are otherwise caught up.
          Hide
          Rohini Palaniswamy added a comment -

          I was taking a look at https://issues.apache.org/jira/browse/MAPREDUCE-5785 and https://issues.apache.org/jira/browse/TEZ-699 .

          MR:

          public static final float DEFAULT_MEMORY_MB_HEAP_RATIO = 1.33f;
          float heapRatio = conf.getFloat(MRJobConfig.MEMORY_MB_HEAP_RATIO,
                    MRJobConfig.DEFAULT_MEMORY_MB_HEAP_RATIO);
          int taskHeapSize =(int)Math.ceil(taskContainerMb / heapRatio);
          public static final float DEFAULT_IO_SORT_MB_HEAP_RATIO = 0.5f;
          ioSortMbPer = JobContext.DEFAULT_IO_SORT_MB_HEAP_RATIO;
          +        }
          +        sortmb = (int)(maxHeapMb * ioSortMbPer);
          

          Tez:

          public static final String TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION =
          +      TEZ_PREFIX + "container.max.java.heap.fraction";
          +  public static final double TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION_DEFAULT = 0.8;
          int maxMemory = (int)(resource.getMemory() * maxHeapFactor);
           

          Few issues, inconsistencies that I see:

          • The MR one is really confusing. For heap it is divide by and io.sort.mb it is multiplication. I think it would be easier to keep both same to avoid confusion. I had to apply more of my grey cells to do the division. Would prefer multiplication to determine the percentage as it is more easy to compute in mind than division.
          • io.sort.mb as 50% of heap seems too high for default value. Most of the pig jobs which have huge bags would start failing.
          • Another issue is taking the defaults now, for a
            4G container – Tez Xmx = 3.2G, MR Xmx=3.0G
            8G container – Tez Xmx = 6.2G, MR xmx = 6G.
            Though the defaults work well for 1 or 2G of memory, for higher memories they seem to be actually wasting a lot of memory considering not more than 500M is usually required for native memory even if the Xmx keeps increasing. We should account that factor into calculation instead of doing Xmx as just a direct percentage of resource.mb.

          Tez settings are usually equivalent of MR settings with an internal mapping to the MR setting taking them if they are specified, so that it is easier for users to switch between frameworks. This is one thing I am seeing inconsistent in terms of how the value is specified and would be good to reconcile both to have same behavior.

          Show
          Rohini Palaniswamy added a comment - I was taking a look at https://issues.apache.org/jira/browse/MAPREDUCE-5785 and https://issues.apache.org/jira/browse/TEZ-699 . MR: public static final float DEFAULT_MEMORY_MB_HEAP_RATIO = 1.33f; float heapRatio = conf.getFloat(MRJobConfig.MEMORY_MB_HEAP_RATIO, MRJobConfig.DEFAULT_MEMORY_MB_HEAP_RATIO); int taskHeapSize =( int ) Math .ceil(taskContainerMb / heapRatio); public static final float DEFAULT_IO_SORT_MB_HEAP_RATIO = 0.5f; ioSortMbPer = JobContext.DEFAULT_IO_SORT_MB_HEAP_RATIO; + } + sortmb = ( int )(maxHeapMb * ioSortMbPer); Tez: public static final String TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION = + TEZ_PREFIX + "container.max.java.heap.fraction" ; + public static final double TEZ_CONTAINER_MAX_JAVA_HEAP_FRACTION_DEFAULT = 0.8; int maxMemory = ( int )(resource.getMemory() * maxHeapFactor); Few issues, inconsistencies that I see: The MR one is really confusing. For heap it is divide by and io.sort.mb it is multiplication. I think it would be easier to keep both same to avoid confusion. I had to apply more of my grey cells to do the division. Would prefer multiplication to determine the percentage as it is more easy to compute in mind than division. io.sort.mb as 50% of heap seems too high for default value. Most of the pig jobs which have huge bags would start failing. Another issue is taking the defaults now, for a 4G container – Tez Xmx = 3.2G, MR Xmx=3.0G 8G container – Tez Xmx = 6.2G, MR xmx = 6G. Though the defaults work well for 1 or 2G of memory, for higher memories they seem to be actually wasting a lot of memory considering not more than 500M is usually required for native memory even if the Xmx keeps increasing. We should account that factor into calculation instead of doing Xmx as just a direct percentage of resource.mb. Tez settings are usually equivalent of MR settings with an internal mapping to the MR setting taking them if they are specified, so that it is easier for users to switch between frameworks. This is one thing I am seeing inconsistent in terms of how the value is specified and would be good to reconcile both to have same behavior.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12644318/MAPREDUCE-5785.v03.patch
          against trunk revision .

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12644318/MAPREDUCE-5785.v03.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4604//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4604//console This message is automatically generated.
          Hide
          Gera Shegalov added a comment -

          Thanks for review, Jason! Here is v03 patch.

          Show
          Gera Shegalov added a comment - Thanks for review, Jason! Here is v03 patch.
          Hide
          Jason Lowe added a comment -

          Briefly looked at the new patch, a few comments:

          • I thought MAPREDUCE-5028 solved the integer sign overflow issues, so do we still need to limit it to 1024 instead of 2047?
          • Nit: IO_SORT_MB_PERCENTAGE is actually treated as a ratio rather than a percentage, otherwise users may try to set it to something like 50 rather than 0.5 based on the description. It also doesn't say what it's relative to, so maybe IO_SORT_HEAP_RATIO/mapreduce.task.io.sort.heap.ratio or IO_SORT_MB_HEAP_RATIO/mapreduce.task.io.sort.mb.heap.ratio would be more clear and consistent with the other property?
          • mapred-default description of mapreduce.task.io.sort.mb has "perecentage"
          • Technically this may need to be marked as an incompatible change, as jobs that were setting an explicit large heap due to their particular code needs but not setting any value for io.sort.mb may now fail with OOM errors since they will implicitly get a smaller heap due to an automatically enlarged io.sort.mb.
          Show
          Jason Lowe added a comment - Briefly looked at the new patch, a few comments: I thought MAPREDUCE-5028 solved the integer sign overflow issues, so do we still need to limit it to 1024 instead of 2047? Nit: IO_SORT_MB_PERCENTAGE is actually treated as a ratio rather than a percentage, otherwise users may try to set it to something like 50 rather than 0.5 based on the description. It also doesn't say what it's relative to, so maybe IO_SORT_HEAP_RATIO/mapreduce.task.io.sort.heap.ratio or IO_SORT_MB_HEAP_RATIO/mapreduce.task.io.sort.mb.heap.ratio would be more clear and consistent with the other property? mapred-default description of mapreduce.task.io.sort.mb has "perecentage" Technically this may need to be marked as an incompatible change, as jobs that were setting an explicit large heap due to their particular code needs but not setting any value for io.sort.mb may now fail with OOM errors since they will implicitly get a smaller heap due to an automatically enlarged io.sort.mb.
          Hide
          Gera Shegalov added a comment -

          Karthik Kambatla, can you take a look regarding your suggestion of including sort buffer size?

          Show
          Gera Shegalov added a comment - Karthik Kambatla , can you take a look regarding your suggestion of including sort buffer size?
          Hide
          Gera Shegalov added a comment -

          The test passes on my notebook. The failing part of the test is Xmx3000m with io.sort.mb=2000, i.e, where actually that much memory is requested. The build machine was not able to handle it. Independently of this, I would like to hear comments on the approach.

          Show
          Gera Shegalov added a comment - The test passes on my notebook. The failing part of the test is Xmx3000m with io.sort.mb=2000, i.e, where actually that much memory is requested. The build machine was not able to handle it. Independently of this, I would like to hear comments on the approach.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

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

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

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

          -1 core tests. The patch failed these unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core:

          org.apache.hadoop.mapreduce.v2.app.job.impl.TestMapReduceChildJVM

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636218/MAPREDUCE-5785.v02.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core: org.apache.hadoop.mapreduce.v2.app.job.impl.TestMapReduceChildJVM +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4457//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4457//console This message is automatically generated.
          Hide
          Gera Shegalov added a comment -

          Lohit Vijayarenu suggested offline to consider modifying the conf on the client-side during job submission. It definitely works well for Xmx. io.sort.mb is safer to set on in the MapTask JVM due to Xmx-Runtime.maxMemory discrepancy. Server-side patch does not need to worry about potential multiple client implementations. Thoughts?

          For now, uploading v02 based on the feedback by Jason Lowe and Karthik Kambatla.

          Show
          Gera Shegalov added a comment - Lohit Vijayarenu suggested offline to consider modifying the conf on the client-side during job submission. It definitely works well for Xmx. io.sort.mb is safer to set on in the MapTask JVM due to Xmx-Runtime.maxMemory discrepancy . Server-side patch does not need to worry about potential multiple client implementations. Thoughts? For now, uploading v02 based on the feedback by Jason Lowe and Karthik Kambatla .
          Hide
          Jason Lowe added a comment -

          Haven't had a chance to look into the patch into great detail, but here are some initial comments:

          • should 'memory.mb.xmx.ratio' be 'memory.mb.heap.ratio'? Even the code names it that internally.
          • rather than commenting out the mapred-default property it should leave it in without a value set. See the mapred.child.env entry as an example.
          • should be easy to add a unit test that verifies the ratio is working as intended, e.g.: changing it sees a corresponding jvm argument change out of MapReduceChildJVM.getVMCommand and setting an explicit heap setting in the config prevents the ratio from taking effect.
          Show
          Jason Lowe added a comment - Haven't had a chance to look into the patch into great detail, but here are some initial comments: should 'memory.mb.xmx.ratio' be 'memory.mb.heap.ratio'? Even the code names it that internally. rather than commenting out the mapred-default property it should leave it in without a value set. See the mapred.child.env entry as an example. should be easy to add a unit test that verifies the ratio is working as intended, e.g.: changing it sees a corresponding jvm argument change out of MapReduceChildJVM.getVMCommand and setting an explicit heap setting in the config prevents the ratio from taking effect.
          Hide
          Gera Shegalov added a comment -

          Sandy Ryza, please elaborate regarding increased chances of OOM. Currently, if the users have not tuned Xmx, they'll get 200m. With the patch, the'll get 770m. If the user specified map reduce.*.memory.mb smaller than the default 1024 (also minimum-allocation-mb), I don't allow the Xmx be lower than the previous default Xmx200m in the patch.

          Regarding the reversal of which parameter controls the other, I can see it either way. Your point works for me. But it is also convenient to explicitly state the cap for the container. The latter seems to be more backwards-compatible.

          Show
          Gera Shegalov added a comment - Sandy Ryza , please elaborate regarding increased chances of OOM. Currently, if the users have not tuned Xmx, they'll get 200m. With the patch, the'll get 770m. If the user specified map reduce.*.memory.mb smaller than the default 1024 (also minimum-allocation-mb), I don't allow the Xmx be lower than the previous default Xmx200m in the patch. Regarding the reversal of which parameter controls the other, I can see it either way. Your point works for me. But it is also convenient to explicitly state the cap for the container. The latter seems to be more backwards-compatible.
          Hide
          Gera Shegalov added a comment -

          Karthik Kambatla, the constant headroom has been discussed. My thinking in favor of percentage-kind of overhead is that

          • it's easy to reason about direct proportional overhead
          • reasons a larger container size is specified is to process more data. Side effects of it is that some code is executed more frequently and more byte code is compiled into native. The more native memory is used through the NIO stack, and native compressor libraries. The more tracking structures a GC might have.

          I like your idea to tune io.sort.mb accordingly. I'd pick the default 50% of Xmx to match the current defaults: io.sort.mb=100 and -Xmx200m. I'll add this to the patch.

          Show
          Gera Shegalov added a comment - Karthik Kambatla , the constant headroom has been discussed. My thinking in favor of percentage-kind of overhead is that it's easy to reason about direct proportional overhead reasons a larger container size is specified is to process more data. Side effects of it is that some code is executed more frequently and more byte code is compiled into native. The more native memory is used through the NIO stack, and native compressor libraries. The more tracking structures a GC might have. I like your idea to tune io.sort.mb accordingly. I'd pick the default 50% of Xmx to match the current defaults: io.sort.mb=100 and -Xmx200m. I'll add this to the patch.
          Hide
          Sandy Ryza added a comment -

          Something like this has been long-needed.

          Though I'm worried that it's not backwards-compatible - users would see their JVM max heaps change in certain situations. In situations where they didn't set the max heap, were cutting it close, but were still OK, they could see OutOfMemoryErrors after the change.

          Another thing is that, as a user, I care more about my max heap size than how much I request from YARN. The latter is usually a consequence of the former.

          One possible way around both of these would be to add a new parameter that controls max heap size and sets mapreduce.*.memory.mb accordingly.

          Show
          Sandy Ryza added a comment - Something like this has been long-needed. Though I'm worried that it's not backwards-compatible - users would see their JVM max heaps change in certain situations. In situations where they didn't set the max heap, were cutting it close, but were still OK, they could see OutOfMemoryErrors after the change. Another thing is that, as a user, I care more about my max heap size than how much I request from YARN. The latter is usually a consequence of the former. One possible way around both of these would be to add a new parameter that controls max heap size and sets mapreduce.*.memory.mb accordingly.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Good idea. Thanks for filing and working on this, Gera Shegalov. What are your thoughts on having a constant headroom (say, 200 MB) instead of a ratio? Also, it would be nice to pick mapreduce.task.io.sort.mb automatically using a ratio, by default. Would be nice to do that too.

          Show
          Karthik Kambatla (Inactive) added a comment - Good idea. Thanks for filing and working on this, Gera Shegalov . What are your thoughts on having a constant headroom (say, 200 MB) instead of a ratio? Also, it would be nice to pick mapreduce.task.io.sort.mb automatically using a ratio, by default. Would be nice to do that too.
          Hide
          Gera Shegalov added a comment -

          v01 for review

          Show
          Gera Shegalov added a comment - v01 for review

            People

            • Assignee:
              Gera Shegalov
              Reporter:
              Gera Shegalov
            • Votes:
              0 Vote for this issue
              Watchers:
              21 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development