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




core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
 (lines 59 - 61)
<https://reviews.apache.org/r/46390/#comment193129>

    Move to SLAJobEventXCommand



core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
 (line 63)
<https://reviews.apache.org/r/46390/#comment193142>

    updateJobInfo



core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobHistoryXCommand.java
 (lines 55 - 56)
<https://reviews.apache.org/r/46390/#comment193146>

    Should throw exception



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
50)
<https://reviews.apache.org/r/46390/#comment193112>

    Can we avoid leaking slaMap and historySet into the command classes? It is 
not a good programming practice.



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
94)
<https://reviews.apache.org/r/46390/#comment193151>

    Not required. Being logged again in SLACalculatorMemory



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
150)
<https://reviews.apache.org/r/46390/#comment193132>

    Return state from these classes and use that to perform update operations 
the maps in SLACalculatorMemory. Do not update them here.



core/src/main/java/org/apache/oozie/command/sla/SLAJobHistoryXCommand.java 
(line 93)
<https://reviews.apache.org/r/46390/#comment193144>

    the x exception - Comment is meaningless. Please remove that in all places.



core/src/main/java/org/apache/oozie/command/sla/SLAJobHistoryXCommand.java 
(line 95)
<https://reviews.apache.org/r/46390/#comment193143>

    updateSLASummary



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 127)
<https://reviews.apache.org/r/46390/#comment193153>

    LOG.warn



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 131)
<https://reviews.apache.org/r/46390/#comment193116>

    Failed to fetch the job (remove workflow)



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 147)
<https://reviews.apache.org/r/46390/#comment193120>

    adding -> Adding
    
    getEventProcessed -> EventProcessed



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java 
<https://reviews.apache.org/r/46390/#comment193122>

    If this is removed, history set will not be updated for end time and start 
time until 1 day (when HistoryPurgeWorker runs) if job completion event 
happened when this server went down or was going down.



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 223)
<https://reviews.apache.org/r/46390/#comment193123>

    Remove double spaces in sentence 
    
    " Starting updateJobSla  for job -> "Starting updateJobSla for job



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 241)
<https://reviews.apache.org/r/46390/#comment193124>

    job has SLA event change



core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java (line 472)
<https://reviews.apache.org/r/46390/#comment193128>

    Method always returns true which is not correct



core/src/main/java/org/apache/oozie/sla/SLAXCommandFactory.java (lines 56 - 57)
<https://reviews.apache.org/r/46390/#comment193114>

    Formatting - Should be in braces



core/src/main/java/org/apache/oozie/sla/SLAXCommandFactory.java (lines 81 - 82)
<https://reviews.apache.org/r/46390/#comment193113>

    Formatting - Should be in braces


This patch adds too much logging. Please review and cut down on logging.

- Rohini Palaniswamy


On April 19, 2016, 8:54 p.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46390/
> -----------------------------------------------------------
> 
> (Updated April 19, 2016, 8:54 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2509
>     https://issues.apache.org/jira/browse/OOZIE-2509
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> SLA job status can stuck in running state
> 
> 
> Diffs
> -----
> 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/command/sla/SLAJobHistoryXCommand.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowActionJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowActionJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java 
> 42313fd3ab804c527afc115402295916b29f63d2 
>   core/src/main/java/org/apache/oozie/sla/SLAXCommandFactory.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/test/java/org/apache/oozie/service/TestHASLAService.java 
> 795db37316d6ced825618f6976133691c02bd940 
>   core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java 
> 432efef6989951b0d66405dd3762cbfe2c965556 
>   core/src/test/java/org/apache/oozie/sla/TestSLAEventGeneration.java 
> 7a710c28cf035e528e7d024f254205d6c52d3309 
>   core/src/test/java/org/apache/oozie/sla/TestSLAJobEventListener.java 
> ebb12f77552f26aba962dddc63e3158d007b07a4 
>   core/src/test/java/org/apache/oozie/sla/TestSLAService.java 
> c3bc110d825f4e3b9a1afe1becd4b045e8c6dacf 
> 
> Diff: https://reviews.apache.org/r/46390/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to