[ 
https://issues.apache.org/jira/browse/OOZIE-616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204860#comment-13204860
 ] 

[email protected] commented on OOZIE-616:
-----------------------------------------------------


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


The error handling needs improvement as exceptions are not chained.


trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/3143/#comment10959>

    Exceptions should be chained to prevent loss of details. Any reason to not 
chain the exception here?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10975>

    Can you add a description of the class?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10968>

    Can we chain the exception?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10969>

    Can we chain the exception?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10970>

    Lack of details will hurt debugging - please include the appropriate error 
message from the previous line.



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10971>

    Lack of details will hurt debugging - please include the appropriate error 
message from the previous line.



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10972>

    Lack of details will hurt debugging - please include the appropriate error 
message from the previous line.



trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
<https://reviews.apache.org/r/3143/#comment10973>

    Can you chain the exception here?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java
<https://reviews.apache.org/r/3143/#comment10976>

    Can you add a description of the class?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java
<https://reviews.apache.org/r/3143/#comment10978>

    The error message may not be representative of the cause since Exception is 
being caught. If the error message is to be retained then try catching the 
appropriate exception thrown by getDocumentFromXML, i.e., 
ParserConfigurationException, SAXException, IOException



trunk/core/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java
<https://reviews.apache.org/r/3143/#comment10977>

    Can you chain the exception?



trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10983>

    Similar to the tests for PrepareActionsDriver, delete the directory prior 
to the creation.



trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10979>

    A better test would be to catch LauncherException and check for a 
reasonable error message and fail for Exception - something like:
    
    } catch (LauncherException le) {
       assertEquals(<check for message>);
       return;
    } catch (Exception ex) {
       <fail here>
    }
    
    <fail here too> if no exception was thrown



trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10980>

    A better test would be to catch LauncherException and check for a 
reasonable error message and fail for Exception - something like:
    
    } catch (LauncherException le) {
       assertEquals(<check for message>);
       return;
    } catch (Exception ex) {
       <fail here>
    }
    
    <fail here too> if no exception was thrown



trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFileSystemActions.java
<https://reviews.apache.org/r/3143/#comment10981>

    This code looks like a copy/paste of the method from PrepareActionsDriver - 
can this be refactored to avoid the copy?



trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPrepareActionsDriver.java
<https://reviews.apache.org/r/3143/#comment10982>

    A better test would be to catch LauncherException and check for a 
reasonable error message and fail for Exception - something like:
    
    } catch (LauncherException le) {
       assertEquals(<check for message>);
    } catch (Exception ex) {
       <fail here>
    }


- Santhosh


On 2012-02-08 21:06:03, Kiran Nagasubramanian wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3143/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-02-08 21:06:03)
bq.  
bq.  
bq.  Review request for oozie, Mohammad Islam and Angelo K. Huang.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  a) Separate classes for different types of actions
bq.     ------------------------------------------------
bq.     The different types of actions like FS actions, HCat related actions, 
etc. can be grouped together in separate classes like FSActions, HCatActions, 
etc.
bq.  
bq.  b) Passing the prepare logic to the Launcher
bq.     -----------------------------------------
bq.     Launcher needs the Prepare XML block to execute the actions. Oozie 
server can write the XML block to a file on DFS and then the Launcher could 
read from there.
bq.  
bq.  c) Execution of actions through a "Driver"
bq.     ---------------------------------------
bq.     The Launcher can pass the XML block to the Driver which parses the 
content and calls corresponding methods that are grouped in different classes.
bq.  
bq.  
bq.  This addresses bug OOZIE-616.
bq.      https://issues.apache.org/jira/browse/OOZIE-616
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
trunk/core/src/main/java/org/apache/oozie/action/hadoop/FileSystemActions.java 
PRE-CREATION 
bq.    
trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
1213901 
bq.    
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 
1213901 
bq.    
trunk/core/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java
 PRE-CREATION 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFileSystemActions.java
 PRE-CREATION 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
 1213901 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 
1213901 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionError.java
 1213901 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
 1213901 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java
 1213901 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPrepareActionsDriver.java
 PRE-CREATION 
bq.    
trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
 1213901 
bq.  
bq.  Diff: https://reviews.apache.org/r/3143/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Yes
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kiran
bq.  
bq.


                
> Moving action prepare FS execution to LauncherMapper
> ----------------------------------------------------
>
>                 Key: OOZIE-616
>                 URL: https://issues.apache.org/jira/browse/OOZIE-616
>             Project: Oozie
>          Issue Type: Improvement
>            Reporter: Kiran Nagasubramanian
>
> 1) Motivations:
>     ----------------
> a) Currently Oozie Server executes the Prepare logic before posting the 
> Launcher job to Hadoop. If the launcher fails and the execution is retried by 
> Hadoop automatically, the prepare logic is not re-executed. So, for 
> Java/MR/Pig actions, if the launcher fails and it is retried by Hadoop 
> automatically, the prepare logic is not executed again. Once the prepare 
> logic execution is moved to the Launcher, the prepare logic is also 
> re-executed during every retry.
> b) Heavy duty operations like copy that are currently not supported can be 
> supported once the prepare logic has been moved to the Launcher.
> 2) Design choices for the proposed model:
>      ------------------------------------------------
> a) Separate classes for different types of actions
>    ---------------------------------------------------------
>    The different types of actions like FS actions, HCat related actions, etc. 
> can be grouped together in separate classes like FSActions, HCatActions, etc.
> b) Passing the prepare logic to the Launcher
>    ---------------------------------------------------
> Launcher needs the Prepare XML block to execute the actions. Oozie server can 
> write the XML block to a file on DFS and then the Launcher could read from 
> there.
> c) Execution of actions through a "Driver"
>    -----------------------------------------------
>    The Launcher can pass the XML block to the Driver which parses the content 
> and calls corresponding methods that are grouped in different classes.
> 3) Pros and Cons of the design change:
>    --------------------------------------------
> a) User facing impacts:
>   -------------------------
> i) At present: In case of prepare block failure,  there is no impact on the 
> retry of map reduce job since the prepare block would have got executed 
> earlier itself. The impact is seen only when Launcher     fails.
>                 
>   As per the proposed design: When the prepare block fails it will impact the 
> retry of the map reduce job.
>                 
>                 ii) At present: In the case of prepare block failure, the 
> launcher mapper is not launched at all.
>                 
>                     As per the proposed design: The cost of executing the 
> prepare block would go up since it requires the launch of a map only task. 
> This is highly pronounced in case of failures
>                                 
> Thanks a lot for all the input from Oozie team members.
> Please post your suggestions. Thanks.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to