> 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
- 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 > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/XCommand.java, line 524 > > <https://reviews.apache.org/r/9602/diff/7/?file=276732#file276732line524> > > > > It is better to evaluate eventsConfigured only once rather than during > > each method call > > how about the following in Xcommand; > > > > protected static final boolean eventsConfigured = > > Services.get().get(EventHandlerService.class) != null; thnx > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java, > > line 50 > > <https://reviews.apache.org/r/9602/diff/7/?file=276748#file276748line50> > > > > is there any need for this? This just seems like user level suspend. > > ErrorCode and error message should be null okay > 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. 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 > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java, > > line 83 > > <https://reviews.apache.org/r/9602/diff/7/?file=276760#file276760line83> > > > > the default value for CONF_LISTENERS should be already existing > > implementation of job and sla event listeners > > > > conf.getClasses(CONF_LISTENERS, jobJMSEvnetListener.class); I mentioned in earlier comment that this change should be part of your JMS patch, otherwise I need to apply part of your patch to get the jmsjobeventlistener > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java, > > line 90 > > <https://reviews.apache.org/r/9602/diff/7/?file=276765#file276765line90> > > > > assert for the expected attributes like status, errorcode and others, > > so we know for sure that events are generated with correct attributes. > > same comment for all tests ok todo > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java, > > line 106 > > <https://reviews.apache.org/r/9602/diff/7/?file=276733#file276733line106> > > > > I think its better to filter to do > > if (eventsConfigured) {generateEvent;} > > > > instead of filtering in generateEvent() method. That will save one > > function call. ok TODO > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java, > > line 215 > > <https://reviews.apache.org/r/9602/diff/7/?file=276734#file276734line215> > > > > It may happen that missingDeps is null but push missing deps exist, so > > we still want to generate event > > can we do > > if (coordAction.getStatus()!=READY)? > > let me check > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, > > line 255 > > <https://reviews.apache.org/r/9602/diff/7/?file=276741#file276741line255> > > > > should be generated if action not in ready checking > On April 1, 2013, 5:39 a.m., Virag Kothari wrote: > > trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java, > > line 229 > > <https://reviews.apache.org/r/9602/diff/7/?file=276744#file276744line229> > > > > ActionCheckX is called repeatedly by a service. It should only generate > > event when some status has changed. good point. todo - Mona ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9602/#review18571 ----------------------------------------------------------- On March 31, 2013, 6:07 p.m., Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9602/ > ----------------------------------------------------------- > > (Updated March 31, 2013, 6:07 p.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/CoordKillXCommand.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/DummyJobEventListener.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 > > Diff: https://reviews.apache.org/r/9602/diff/ > > > Testing > ------- > > unit tests added > > > Thanks, > > Mona Chitnis > >
