-----------------------------------------------------------
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
> 
>

Reply via email to