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


Overall clean implementations.
We need to give higher priority to all the high priority command.
Consider a case: where there is only one ChangeCommand for a job. but there are 
other commands from other jobs. If change command doesn't have the higher 
priority, it have wait for its turn. Highest priority will give it a better 
chance.

Kill command is already high priority (2) command (See CoordKillXCommand). We 
need to do the similar things for change/suspend/pause commands. Also make sure 
the same is true for WF/Coord/Bundle*XCommand level.


http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
<https://reviews.apache.org/r/3030/#comment8484>

    ConcurrentHashMap might be better data structure.



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
<https://reviews.apache.org/r/3030/#comment8485>

    extra space



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
<https://reviews.apache.org/r/3030/#comment8486>

    minor thing : type exit-->exist



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
<https://reviews.apache.org/r/3030/#comment8487>

    the whole list is being used. so the the list for specific lockKey should 
be removed. I think code is right. the comments needs to be updated.



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
<https://reviews.apache.org/r/3030/#comment8488>

    the log level should be "warn" instead of "trace"



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/util/PollablePriorityDelayQueue.java
<https://reviews.apache.org/r/3030/#comment8489>

    The code between line 53-59 and 72-79 looks very similar. Can we make it to 
a separate method and call it from this two places? 



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml
<https://reviews.apache.org/r/3030/#comment8483>

    What about suspend and pause command?


- Mohammad


On 2011-12-06 21:31:34, Mohamed Battisha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3030/
> -----------------------------------------------------------
> 
> (Updated 2011-12-06 21:31:34)
> 
> 
> Review request for oozie.
> 
> 
> Summary
> -------
> 
> Oozie continues to materialize new actions after end date modification. The 
> main issue is related how Oozie materialized the actions based on a FIFO 
> priority queue. Changing the status of a bundle/coordinator job should take 
> higher priority than executing this job. 
> 
> The main idea is to enable the queue to handle interruptions. Mainly, once 
> you finished what you are working on currently, you should focus on executing 
> this next action. 
> 
> The issue can be illustrated as follow: 
> 
> 1. Configure a pipeline to run for 1 hour 
> 2. Start the pipeline 
> 3. After it starts materializing new actions, change the end time (in my 
> example - to 10 minutes after the pipeline 
> starts) 
> 4. Monitor the coordinator apps - they will continue to materialize new 
> actions past the end time.
> 
> 
> This addresses bug OOZIE-591.
>     https://issues.apache.org/jira/browse/OOZIE-591
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/Command.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/XCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleJobResumeXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundlePauseXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleRerunXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleStatusUpdateXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/command/TestCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/util/XCallable.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/bundle/BundleUnpauseXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionNotificationXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionReadyXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPauseXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordResumeXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSuspendXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordUnpauseXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/command/wf/WfEndXCommand.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallableQueueService.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/RecoveryService.java
>  1209829 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/util/PollablePriorityDelayQueue.java
>  1209829 
> 
> Diff: https://reviews.apache.org/r/3030/diff
> 
> 
> Testing
> -------
> 
> Regression for all the Commands in addition for specific testing for the 
> Interrupt Driven Map
> 
> 
> Thanks,
> 
> Mohamed
> 
>

Reply via email to