----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17720/#review38769 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/executor/jpa/SLARegistrationQueryExecutor.java <https://reviews.apache.org/r/17720/#comment71047> Can we have corresponding JPAExecutor classes deleted? core/src/main/java/org/apache/oozie/executor/jpa/SLASummaryQueryExecutor.java <https://reviews.apache.org/r/17720/#comment71048> Can we have corresponding JPAExecutor classes deleted? core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71278> What is the purpose of locking while loading on restart? This is very bad for startup time. core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71279> It should still be synchronized on slaCalc. Please avoid this whole locks logic if it is only a single server. You don't need the lock overhead when Oozie is not running in HA mode. Also please do not acquire locks for the whole processing. Only if you are going to do some update(confirmwithDB), then acquire lock. And if you fail to acquire lock, just continue. In the next periodic run when you check DB you can remove from the map if other server has updated it. core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71297> Why construct a summaryBean object only if there is eventProcessed set in it. Can't we retrieve and return just that one column? Also this should be done only if we go into confirmWithDB core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71287> If the SLA is already processed, you should be setting the actual times and not adding to the slaMap. Only if not already processed you should be adding to slaMap core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71289> Check the eventProc value. What if it is already processed? core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java <https://reviews.apache.org/r/17720/#comment71290> Please revert all this isQueued logic in SLA code. We knew this when we put some memory limit for the queue. The queue needs to be fixed to spill overflowing elements to disk and process from there so that it is handled generically for both jobs and sla notifications. Putting isQueued logic makes it more complicated here and can introduce bugs and also slow down the system really bad if queue is full and we keep making more DB calls for SLA checks. core/src/main/java/org/apache/oozie/sla/SLARegistrationBean.java <https://reviews.apache.org/r/17720/#comment71294> When is apptype null? core/src/test/java/org/apache/oozie/service/TestHASLAService.java <https://reviews.apache.org/r/17720/#comment71298> Can we add more test cases to test all HA scenarios? - Rohini Palaniswamy On March 17, 2014, 7:05 p.m., Ryota Egashira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17720/ > ----------------------------------------------------------- > > (Updated March 17, 2014, 7:05 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1678 > https://issues.apache.org/jira/browse/OOZIE-1678 > > > Repository: oozie-git > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1678 > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/event/EventQueue.java 39156d6 > core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java f32afb4 > > core/src/main/java/org/apache/oozie/executor/jpa/SLARegistrationQueryExecutor.java > e3b115f > > core/src/main/java/org/apache/oozie/executor/jpa/SLASummaryQueryExecutor.java > 79d11ed > core/src/main/java/org/apache/oozie/service/EventHandlerService.java > 4207a07 > core/src/main/java/org/apache/oozie/service/JPAService.java aba8709 > core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java ea53712 > core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java 618d899 > core/src/main/java/org/apache/oozie/sla/SLARegistrationBean.java a2260a4 > core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java 0a70326 > core/src/main/java/org/apache/oozie/sla/service/SLAService.java 2458e69 > > core/src/test/java/org/apache/oozie/executor/jpa/TestSLARegistrationQueryExecutor.java > 00fb677 > > core/src/test/java/org/apache/oozie/executor/jpa/TestSLASummaryQueryExecutor.java > 2e170a4 > core/src/test/java/org/apache/oozie/service/TestHASLAService.java > PRE-CREATION > core/src/test/java/org/apache/oozie/sla/TestSLAEventGeneration.java f3bfc29 > core/src/test/java/org/apache/oozie/test/ZKXTestCase.java 7bebaf0 > core/src/test/resources/coord-action-sla.xml e88df6c > > Diff: https://reviews.apache.org/r/17720/diff/ > > > Testing > ------- > > > Thanks, > > Ryota Egashira > >
