> On April 1, 2013, 5:39 a.m., Virag Kothari wrote:
> > - SuspendX.suspend() should generate event for wf suspended state. This 
> > method is called when a transient error occurs and wf goes to SUSPENDED.
> > 
> > - ActionCheckX.failAction() should generate wf Failed event.
> > 
> > - CoordActionUpdatePushMissingDependency should generate waiting event
> > 
> > - It seems that at some places like coordkill, coordinator job events are 
> > generated. But it seems at many other places, commands to generate coord 
> > job events are missed. If that is the case, plz remove event generating 
> > code for coordinator job and wf action. Can you only generate events for 
> > Workflow job and coordinator action in this patch and remove all other 
> > unused/commented code?
> > 
> > - Have Javadoc
> 
> Mona Chitnis wrote:
>     - added generate event in SuspendX.suspendJob(). I was a little doubtful 
> about this place because it does not do a DB update on the WF Job in/after 
> the suspendJob() method.
>     
>     - In ActionCheckX , generateEvent is called in finally block, so failed 
> wf case is handled
>     
>     - CoordActionUpdatePushMissingDependency calls 
> CoordPushDependencyCheckXCommand.updateAction() and generateEvent is done 
> there

discussed offline, ActionCheckX needs to generate event only in failure cases


> On April 1, 2013, 5:39 a.m., Virag Kothari wrote:
> > trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java, line 
> > 100
> > <https://reviews.apache.org/r/9602/diff/7/?file=276754#file276754line100>
> >
> >     I dont see the requirement for locking as the queue and currentsize is 
> > threadsafe and the combined operation need not be threadsafe in case of 
> > polling.
> 
> Mona Chitnis wrote:
>     The for loop execution for batchsize is assured to be correct if 
> synchronized. Else one case I can think of different threads updating the 
> index towards batchsize while polling events, and as a result, each thread 
> polling more/less than batchSize events. I followed this pattern from our 
> PriorityDelayQueue logic for Callables

discussed offline..no need for locking as batchsize is readonly variable and 
the index is local.


- Virag


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


On April 1, 2013, 10:23 a.m., Mona Chitnis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9602/
> -----------------------------------------------------------
> 
> (Updated April 1, 2013, 10:23 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1209
> WIP patch
> 
> 
> This addresses bug OOZIE-1209.
>     https://issues.apache.org/jira/browse/OOZIE-1209
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/java/org/apache/oozie/client/SLAEvent.java 1462882 
>   trunk/client/src/main/java/org/apache/oozie/client/event/Event.java 
> PRE-CREATION 
>   trunk/client/src/main/java/org/apache/oozie/client/event/JobEvent.java 
> PRE-CREATION 
>   trunk/client/src/main/java/org/apache/oozie/client/event/SLAEvent.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/client/rest/JsonCoordinatorAction.java
>  1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/XCommand.java 1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionReadyXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java
>  1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/wf/KillXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java 
> 1462882 
>   trunk/core/src/main/java/org/apache/oozie/event/BundleJobEvent.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/EventQueue.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/event/WorkflowJobEvent.java 
> PRE-CREATION 
>   
> trunk/core/src/main/java/org/apache/oozie/event/listener/JobEventListener.java
>  PRE-CREATION 
>   
> trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java
>  PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java 
> PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/service/RecoveryService.java 
> 1462882 
>   
> trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java
>  PRE-CREATION 
>   trunk/core/src/main/resources/oozie-default.xml 1462882 
>   
> trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java
>  1462882 
>   trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java 
> PRE-CREATION 
>   trunk/core/src/test/java/org/apache/oozie/event/TestEventQueue.java 
> PRE-CREATION 
>   
> trunk/core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java
>  PRE-CREATION 
>   trunk/core/src/test/java/org/apache/oozie/test/XEventTestCase.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9602/diff/
> 
> 
> Testing
> -------
> 
> unit tests added
> 
> 
> Thanks,
> 
> Mona Chitnis
> 
>

Reply via email to