Re: Review Request 12973: [OOZIE-1372] When using uber mode, Oozie should also make the AM container size larger

[Rohini Palaniswamy <ro...@gmail.com>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12973/#review25042
-----------------------------------------------------------



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49367>

    Configuration.addDeprecation("mapred.job.map.memory.mb", 
          new String[] {MRJobConfig.MAP_MEMORY_MB});
        Configuration.addDeprecation("mapred.map.child.env", 
          new String[] {MRJobConfig.MAP_ENV});
        Configuration.addDeprecation("mapred.map.child.java.opts", 
          new String[] {MRJobConfig.MAP_JAVA_OPTS});
    
    There is deprecation for everything other than mapred.child.java.opts. So it is enough if we check for the mapreduce.* for those that have deprecation. 
    
    For the deprecation to take effect, org.apache.hadoop.mapreduce.util.ConfigUtil should have loaded the deprecation keys. This happens when JobClient or JobConf is instantiated. In our code JobConf is instantiated before, so it is not a problem. 



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49366>

    white space



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49379>

    This should not exceed 4096. Max for 32 bit jvm. But in future we will have 64 bit jvms. So if launcherMapMemoryMB or amMemoryMB itself is > 4096 set it greater than 4096 else limit to 4096.  Same for Xmx



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49368>

    Check only for mapreduce.* values except for mapred.child.java.opts



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49196>

    Redundant to pass a constant as a argument to a method. Pattern should be static variable. pattern compilation is costly and is generally declared as a static variable if you are compiling a static string. 



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49380>

    This should not exceed max 3584 (32 bit jvm limit) if user has not defined greater than 4G (future 64-bit support)



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49376>

    AM opts followed by map opts so that map opts take effect



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49375>

    Just check for mapreduce.*



trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/12973/#comment49374>

    Should be reversed. Map env should be last so that it takes effect


- Rohini Palaniswamy


On July 26, 2013, 5:41 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12973/
> -----------------------------------------------------------
> 
> (Updated July 26, 2013, 5:41 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1372
>     https://issues.apache.org/jira/browse/OOZIE-1372
> 
> 
> Repository: oozie
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1372
> 
> 
> Diffs
> -----
> 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 1505822 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 1505822 
> 
> Diff: https://reviews.apache.org/r/12973/diff/
> 
> 
> Testing
> -------
> 
> -unit test against 2.2.0
> -ran example workflow (uber mode) in QE cluster with Hadoop 2.2.0, and confirmed it succeeded and all job conf set properly
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>