----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9602/#review18571 -----------------------------------------------------------
- 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 trunk/core/src/main/java/org/apache/oozie/command/XCommand.java <https://reviews.apache.org/r/9602/#comment38934> 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; trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java <https://reviews.apache.org/r/9602/#comment38946> I think its better to filter to do if (eventsConfigured) {generateEvent;} instead of filtering in generateEvent() method. That will save one function call. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java <https://reviews.apache.org/r/9602/#comment38947> 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)? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java <https://reviews.apache.org/r/9602/#comment38948> should be generated if action not in ready trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java <https://reviews.apache.org/r/9602/#comment38949> ActionCheckX is called repeatedly by a service. It should only generate event when some status has changed. trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java <https://reviews.apache.org/r/9602/#comment38950> is there any need for this? This just seems like user level suspend. ErrorCode and error message should be null trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38952> 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. trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java <https://reviews.apache.org/r/9602/#comment38951> this will throw NPE if no event to poll trunk/core/src/main/java/org/apache/oozie/event/listener/DummyJobEventListener.java <https://reviews.apache.org/r/9602/#comment38953> if its required for unit testing, can you make this a inner class in the test case trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/9602/#comment38954> the default value for CONF_LISTENERS should be already existing implementation of job and sla event listeners conf.getClasses(CONF_LISTENERS, jobJMSEvnetListener.class); trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java <https://reviews.apache.org/r/9602/#comment38955> 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 trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java <https://reviews.apache.org/r/9602/#comment38956> Junit expectes the first parameter of assertEquals to be the expected value and the next one to be actual - Virag Kothari 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 > >
