----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10569/#review20638 -----------------------------------------------------------
trunk/client/src/main/resources/oozie-sla-0.2.xsd <https://reviews.apache.org/r/10569/#comment42562> why are should_start and should end required.? For example, I should be fine if my job ends by 1 PM. Doesn't care when it started. no need for should_start here. trunk/core/src/main/java/org/apache/oozie/client/rest/sla/JsonSLARegistrationEvent.java <https://reviews.apache.org/r/10569/#comment42563> Will it be of type blob? trunk/core/src/main/java/org/apache/oozie/client/rest/sla/JsonSLARegistrationEvent.java <https://reviews.apache.org/r/10569/#comment42565> Not clear what is string format of the config? will there be any "{". trunk/core/src/main/java/org/apache/oozie/client/rest/sla/JsonSLARegistrationEvent.java <https://reviews.apache.org/r/10569/#comment42564> check if pair.length == 2. otherwise NPE could occurs. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java <https://reviews.apache.org/r/10569/#comment42566> deprecated ? what will be the replacement trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionMaterializeCommand.java <https://reviews.apache.org/r/10569/#comment42567> is it writing to SLA_CALCULATION table that will be removed in another JIRA? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordKillXCommand.java <https://reviews.apache.org/r/10569/#comment42568> just to mention. If line 172 succeeds but some exception happened between 173-181, there is no way to rollback. it is ok for the time being! trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSuspendXCommand.java <https://reviews.apache.org/r/10569/#comment42569> preformWrite looks similar in Kill and suspend, is there a way to put it in one place and reuse? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java <https://reviews.apache.org/r/10569/#comment42570> Null checkings are not required? trunk/core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java <https://reviews.apache.org/r/10569/#comment42571> all places, we are passing the EM as a new param. Can't it be get within genenrateEvent(..) instead. trunk/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java <https://reviews.apache.org/r/10569/#comment42572> Why do we need to create action ID here? It is already generated in another place. Wandering if we are creating multiple ids for one action. trunk/core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java <https://reviews.apache.org/r/10569/#comment42573> As mentioned before, in place of passing "em" as arguments, can't we get JPAService.getEM() in EventService.queueEvent()? No need to pass the same argument is all the places. trunk/core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java <https://reviews.apache.org/r/10569/#comment42574> Does "DONE" means "SUCCESS"? Please double check it. trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLACalculationInsertUpdateJPAExecutor.java <https://reviews.apache.org/r/10569/#comment42575> Not reviewing, because it will be removed ultimately, trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLACalculatorGetJPAExecutor.java <https://reviews.apache.org/r/10569/#comment42576> Same. will be removed. no review. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/10569/#comment42577> is it possible to send more specific Exception instead of basic Exception. Trie for other places. trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java <https://reviews.apache.org/r/10569/#comment42578> For "any" reason, if msgType is NOT JOB or SLA, this thing will be an infinite loop. So call iter.next() once outside if. trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorBean.java <https://reviews.apache.org/r/10569/#comment42579> will be removed ultimately? trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/10569/#comment42580> All new classes, please add some comments at least in class level. what is does. who called this class. trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/10569/#comment42581> So we assume, all times are in UTC. right? Also user will provide in absolute or relative to NT. trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/10569/#comment42582> if expected start/end time is null. NPE? trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/10569/#comment42583> what will be in "else" block? trunk/core/src/main/java/org/apache/oozie/sla/SLAOperations.java <https://reviews.apache.org/r/10569/#comment42584> can we check at the beginning of this method and return if disabled? trunk/core/src/main/java/org/apache/oozie/sla/SLAOperations.java <https://reviews.apache.org/r/10569/#comment42585> Move this deep-copy method to class SLACalcStatus. Because if there is any new member added , you will need to modify in one class. trunk/core/src/main/java/org/apache/oozie/sla/SLARegistrationBean.java <https://reviews.apache.org/r/10569/#comment42586> tab - Mohammad Islam On May 16, 2013, 1:23 a.m., Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10569/ > ----------------------------------------------------------- > > (Updated May 16, 2013, 1:23 a.m.) > > > Review request for oozie. > > > Description > ------- > > 1. Revisiting the SLA handling in Oozie > 2. Addition of a calculator service to process sla events in a continuous > fashion > 3. Added new oozie-sla schema v0.2, concise and relevant > > > This addresses bug OOZIE-1244. > https://issues.apache.org/jira/browse/OOZIE-1244 > > > Diffs > ----- > > trunk/client/src/main/java/org/apache/oozie/AppType.java PRE-CREATION > trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java 1482602 > trunk/client/src/main/java/org/apache/oozie/client/JMSConnectionInfo.java > 1482602 > trunk/client/src/main/java/org/apache/oozie/client/SLAEvent.java 1482602 > trunk/client/src/main/java/org/apache/oozie/client/event/Event.java 1482602 > trunk/client/src/main/java/org/apache/oozie/client/event/JobEvent.java > 1482602 > trunk/client/src/main/java/org/apache/oozie/client/event/SLAEvent.java > 1482602 > trunk/client/src/main/java/org/apache/oozie/client/rest/JsonToBean.java > 1482602 > trunk/client/src/main/resources/oozie-coordinator-0.4.xsd 1482602 > trunk/client/src/main/resources/oozie-sla-0.2.xsd PRE-CREATION > trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd 1482602 > trunk/client/src/main/resources/oozie-workflow-0.5.xsd PRE-CREATION > trunk/client/src/test/java/org/apache/oozie/client/rest/TestJsonToBean.java > 1482602 > trunk/core/pom.xml 1482602 > trunk/core/src/main/conf/oozie-site.xml 1482602 > trunk/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/ErrorCode.java 1482602 > trunk/core/src/main/java/org/apache/oozie/SLAEventBean.java 1482602 > trunk/core/src/main/java/org/apache/oozie/client/rest/JsonSLAEvent.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/client/rest/sla/JsonSLARegistrationEvent.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionCheckXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionMaterializeCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdateXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordKillXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSuspendXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordinatorXCommand.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/command/coord/SLAEventsXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/ActionEndXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/KillXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/ResumeXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/event/BundleJobEvent.java 1482602 > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/event/EventQueue.java 1482602 > trunk/core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/event/WorkflowJobEvent.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/event/listener/JobEventListener.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsNotCompletedJPAExecutor.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/SLAEventInsertJPAExecutor.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/SLAEventsGetForFilterJPAExecutor.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/SLAEventsGetForSeqIdJPAExecutor.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/SLAEventsGetJPAExecutor.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLACalculationInsertUpdateJPAExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLACalculatorGetJPAExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLARegistrationGetJPAExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetJPAExecutor.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/jms/JMSJobEventListener.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/service/JMSTopicService.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/service/JPAService.java 1482602 > trunk/core/src/main/java/org/apache/oozie/service/SchedulerService.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/service/WorkflowStoreService.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/servlet/SLAServlet.java 1482602 > trunk/core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLACalculator.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorBean.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLAOperations.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLARegistrationBean.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java > 1482602 > > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEventListener.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAJobEventListener.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/store/SLAStore.java 1482602 > trunk/core/src/main/java/org/apache/oozie/util/XmlUtils.java 1482602 > trunk/core/src/main/java/org/apache/oozie/util/db/SLADbOperations.java > 1482602 > trunk/core/src/main/java/org/apache/oozie/util/db/SLADbXOperations.java > 1482602 > trunk/core/src/main/resources/META-INF/persistence.xml 1482602 > trunk/core/src/main/resources/oozie-default.xml 1482602 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionMaterializeCommand.java > 1482602 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java > 1482602 > trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java > 1482602 > trunk/core/src/test/java/org/apache/oozie/event/TestEventQueue.java 1482602 > > trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestSLAEventsGetJPAExecutor.java > 1482602 > trunk/core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java > 1482602 > > trunk/core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java > 1482602 > trunk/core/src/test/java/org/apache/oozie/service/TestJMSTopicService.java > 1482602 > > trunk/core/src/test/java/org/apache/oozie/sla/TestSLACalculationJPAExecutor.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEventGeneration.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/sla/TestSLAJobEventListener.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/oozie/sla/TestSLARegistrationGetJPAExecutor.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/sla/TestSLAService.java > PRE-CREATION > trunk/core/src/test/java/org/apache/oozie/test/XDataTestCase.java 1482602 > trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1482602 > trunk/core/src/test/resources/coord-action-sla.xml PRE-CREATION > trunk/core/src/test/resources/wf-action-sla.xml PRE-CREATION > trunk/core/src/test/resources/wf-job-sla.xml PRE-CREATION > trunk/examples/src/main/apps/sla/coordinator.xml 1482602 > trunk/examples/src/main/apps/sla/job.properties 1482602 > trunk/examples/src/main/apps/sla/workflow.xml 1482602 > trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java 1482602 > > Diff: https://reviews.apache.org/r/10569/diff/ > > > Testing > ------- > > ongoing > > > Thanks, > > Mona Chitnis > >
