You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Peter Cseh via Review Board <no...@reviews.apache.org> on 2018/03/23 10:50:17 UTC

Review Request 66243: OOZIE-3193 Applications are not killed when submitted via subworkflow

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

Review request for oozie and András Piros.


Bugs: OOZIE-3193
    https://issues.apache.org/jira/browse/OOZIE-3193


Repository: oozie-git


Description
-------

Tags were not generated properly for external child jobs launched from subWFs and they were not killed.


Diffs
-----

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 919509d35aefae8eed60849ca9c1da4e7ea291de 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 122dfd02e17456b9aab924504faee790813fb6a1 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java fdca5706efbc13741477cee186550d07eee9f737 
  core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java e074d482fb520e96de01b605ee8949ea52fd0d84 
  src/main/resources/checkstyle-header.txt 4247452483c0d2b6f337912dff16efd69d5f86fd 
  src/main/resources/checkstyle.xml 68b6cce92a0394d160c57bab11022e839d16b187 


Diff: https://reviews.apache.org/r/66243/diff/1/


Testing
-------

Tested on deployed cluster.
Added Junit test.


Thanks,

Peter Cseh


Re: Review Request 66243: OOZIE-3193 Applications are not killed when submitted via subworkflow

Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66243/#review199874
-----------------------------------------------------------




core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
Line 1766 (original), 1766 (patched)
<https://reviews.apache.org/r/66243/#comment280381>

    I don't really get the difference between `jobConf` and `context.getProtoActionConf`, but I think it's important.
    
    Would you mind having some comments / JUnit examples for real contents?



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 107-120 (patched)
<https://reviews.apache.org/r/66243/#comment280363>

    Extract method `createSleepMapperReducerJobConf()`.



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 129 (patched)
<https://reviews.apache.org/r/66243/#comment280364>

    Some message to standard error?



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 137 (patched)
<https://reviews.apache.org/r/66243/#comment280366>

    Change signature to `writeJobID(Path, JobConf, String)`.



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 138-142 (patched)
<https://reviews.apache.org/r/66243/#comment280365>

    Use try-with-resources for `FileSystem` and `OutputStreamWriter`.



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 145-152 (patched)
<https://reviews.apache.org/r/66243/#comment280367>

    Use instead `writeJobID(Path, JobConf, String)`.



core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
Lines 35 (patched)
<https://reviews.apache.org/r/66243/#comment280368>

    `SLEEP_TIME_MILLIS`



core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
Lines 46-50 (patched)
<https://reviews.apache.org/r/66243/#comment280369>

    Extract method `sleepUninterrupted(long millis, String message)`.



core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
Lines 56-60 (patched)
<https://reviews.apache.org/r/66243/#comment280370>

    Use instead method `sleepUninterrupted(long millis, String message)`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Line 40 (original), 58 (patched)
<https://reviews.apache.org/r/66243/#comment280371>

    Remove `*` import.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 542-543 (patched)
<https://reviews.apache.org/r/66243/#comment280372>

    Extract method `killWorkflow(String id)` and remove comment.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 545-546 (patched)
<https://reviews.apache.org/r/66243/#comment280373>

    Rename `childMRJobApplicationId` to `externalChildJobId` and remove comment.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 554 (patched)
<https://reviews.apache.org/r/66243/#comment280376>

    Cannot decrypt `sack` :(



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 560 (patched)
<https://reviews.apache.org/r/66243/#comment280377>

    Why not wait for `JOB_TIMEOUT`?



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 567 (patched)
<https://reviews.apache.org/r/66243/#comment280375>

    Use `try-with-resources`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 575-576 (patched)
<https://reviews.apache.org/r/66243/#comment280374>

    Rename `ex` to `ignored` and remove comment.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 596-597 (patched)
<https://reviews.apache.org/r/66243/#comment280379>

    Extract local variables `isSubWfRunning` and `isSubWfActionRunning`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 597 (patched)
<https://reviews.apache.org/r/66243/#comment280378>

    Use `getSubWFId()`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 717-719 (patched)
<https://reviews.apache.org/r/66243/#comment280380>

    Use `try-with-resources`.



src/main/resources/checkstyle.xml
Lines 29-32 (original)
<https://reviews.apache.org/r/66243/#comment280382>

    Submit another patch for another JIRA, please.


- András Piros


On March 23, 2018, 10:50 a.m., Peter Cseh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66243/
> -----------------------------------------------------------
> 
> (Updated March 23, 2018, 10:50 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-3193
>     https://issues.apache.org/jira/browse/OOZIE-3193
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Tags were not generated properly for external child jobs launched from subWFs and they were not killed.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/action/ActionExecutor.java 919509d35aefae8eed60849ca9c1da4e7ea291de 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 122dfd02e17456b9aab924504faee790813fb6a1 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java fdca5706efbc13741477cee186550d07eee9f737 
>   core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java PRE-CREATION 
>   core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java e074d482fb520e96de01b605ee8949ea52fd0d84 
> 
> 
> Diff: https://reviews.apache.org/r/66243/diff/2/
> 
> 
> Testing
> -------
> 
> Tested on deployed cluster.
> Added Junit test.
> 
> 
> Thanks,
> 
> Peter Cseh
> 
>