----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13362/#review25008 -----------------------------------------------------------
Please add test cases which validates the done materialization, next materialized/end materialized time stamp, last action time, last action number, etc of the coord job after materialization and nominal time of the coord action materialized instead of just counting the actions materialized. /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49163> Adding up the frequency sequentially in getNextActionMeasureTime instead of multiplying can cause problems with DST transitions. Encountered test case failures when doing CoordELFunctions.coord_currentRange_sync. Refer comment there. Please add test cases to validate that. Same holds inside the while loop /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49152> Why endMatdTime instead of end? It does not take into account the timezone of coord job /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49153> Why jobPauseTime instead of puase? It does not take into account the timezone of coord job /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49166> Shouldn't you be starting with materialization of effStart for cron expression too? Else you might be skipping one materialization. Or does expr.getNextValidTimeAfter(targetDate); returns targetDate itself if it meets cron criteria? /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49167> Need to check for pause time here too /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49162> DST transitions might have problems with this. Refer previous comment /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java <https://reviews.apache.org/r/13362/#comment49164> Set done materialization to true if next materialization time is after coord job end time. This way you can get rid of the new check in StatusTransitService. - Rohini Palaniswamy On Aug. 8, 2013, 6:33 p.m., Bowen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13362/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2013, 6:33 p.m.) > > > Review request for oozie, Robert Kanter and Rohini Palaniswamy. > > > Repository: oozie > > > Description > ------- > > oozie-1306 cron scheduling for coordinator job > > > Diffs > ----- > > /trunk/core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 1511139 > > /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java > 1511139 > > /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1511139 > > /trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobsGetRunningPastEndtimeJPAExecutor.java > PRE-CREATION > > /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java > 1511139 > > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java > 1511139 > > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java > 1511139 > > /trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobsGetRunningPastEndtimeJPAExecutor.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/13362/diff/ > > > Testing > ------- > > > Thanks, > > Bowen Zhang > >
