You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Bowen Zhang <bo...@yahoo.com> on 2013/08/07 00:42:13 UTC

Re: Review Request 13362: oozie-1306

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Aug. 6, 2013, 10:42 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/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 

Diff: https://reviews.apache.org/r/13362/diff/


Testing
-------


Thanks,

Bowen Zhang


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 294
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line294>
> >
> >     Wasn't DST transition in 11 March 2012 2am ?
> >     
> >     Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
> >     
> >      Refer to TestCoordELFunctions.testCurrent.
> 
> Bowen Zhang wrote:
>     That's exactly what the UTC time is testing.
> 
> Rohini Palaniswamy wrote:
>     The UTC time should be March 11 8:00-12:00 UTC and not March 10th.
> 
> Bowen Zhang wrote:
>     The DST time for spring 2013 is March 10th, not 11th.
> 
> Bowen Zhang wrote:
>     If you look at UTC time and convert it back to America/Los Angeles time, it's indeed forward and backward by one hour.

Sorry. My mistake. I did not notice 2013. Since the other one was 2012, I was somehow reading this one also as 2012.


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > Previously you had added CoordJobsGetRunningPastEndtimeJPAExecutor to handle cron frequencies which will still have more materializations to go even though it won't create any actions. Had asked to set done materialization to true if next materialization time is after coord job end time. CoordJobsGetRunningPastEndtimeJPAExecutor is removed, but setting done materialization is not in this patch.

It's not needed anymore actually as long as we set the next materialization time correct, we don't need to setDone here.


> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 294
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line294>
> >
> >     Wasn't DST transition in 11 March 2012 2am ?
> >     
> >     Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
> >     
> >      Refer to TestCoordELFunctions.testCurrent.

That's exactly what the UTC time is testing.


> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 319
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line319>
> >
> >     4th November 2am was DST.

Refer to previous comment.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


On Sept. 25, 2013, 2:01 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2013, 2:01 a.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/command/coord/CoordCommandUtils.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 294
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line294>
> >
> >     Wasn't DST transition in 11 March 2012 2am ?
> >     
> >     Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
> >     
> >      Refer to TestCoordELFunctions.testCurrent.
> 
> Bowen Zhang wrote:
>     That's exactly what the UTC time is testing.
> 
> Rohini Palaniswamy wrote:
>     The UTC time should be March 11 8:00-12:00 UTC and not March 10th.

The DST time for spring 2013 is March 10th, not 11th.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 319
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line319>
> >
> >     4th November 2am was DST.
> 
> Bowen Zhang wrote:
>     Refer to previous comment.

You are right. This is fine in terms of UTC.


> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 294
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line294>
> >
> >     Wasn't DST transition in 11 March 2012 2am ?
> >     
> >     Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
> >     
> >      Refer to TestCoordELFunctions.testCurrent.
> 
> Bowen Zhang wrote:
>     That's exactly what the UTC time is testing.

The UTC time should be March 11 8:00-12:00 UTC and not March 10th.


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Oct. 1, 2013, 3:21 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 294
> > <https://reviews.apache.org/r/13362/diff/6/?file=356942#file356942line294>
> >
> >     Wasn't DST transition in 11 March 2012 2am ?
> >     
> >     Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
> >     
> >      Refer to TestCoordELFunctions.testCurrent.
> 
> Bowen Zhang wrote:
>     That's exactly what the UTC time is testing.
> 
> Rohini Palaniswamy wrote:
>     The UTC time should be March 11 8:00-12:00 UTC and not March 10th.
> 
> Bowen Zhang wrote:
>     The DST time for spring 2013 is March 10th, not 11th.

If you look at UTC time and convert it back to America/Los Angeles time, it's indeed forward and backward by one hour.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26557
-----------------------------------------------------------


Previously you had added CoordJobsGetRunningPastEndtimeJPAExecutor to handle cron frequencies which will still have more materializations to go even though it won't create any actions. Had asked to set done materialization to true if next materialization time is after coord job end time. CoordJobsGetRunningPastEndtimeJPAExecutor is removed, but setting done materialization is not in this patch.


/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51794>

    Why is this required as it is already set to nextTime?



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51795>

    Please do not replace imports with *



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51797>

    Wasn't DST transition in 11 March 2012 2am ?
    
    Also on the boundary of transition, we expect it to be forward by one hour in Spring DST and backward by one hour in Winter DST.
    
     Refer to TestCoordELFunctions.testCurrent. 



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51798>

    4th November 2am was DST.


- Rohini Palaniswamy


On Sept. 25, 2013, 2:01 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2013, 2:01 a.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/command/coord/CoordCommandUtils.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Oct. 2, 2013, 7:23 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 303
> > <https://reviews.apache.org/r/13362/diff/7/?file=360298#file360298line303>
> >
> >     Why isn't 12:00 also materialized?

Because the job ends at that time. Similarly, if you have a job starts at 10am and ends at 11am with frequency of ${coord:minutes(10)}, there will be no materialization at 11:00am.


> On Oct. 2, 2013, 7:23 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 329
> > <https://reviews.apache.org/r/13362/diff/7/?file=360298#file360298line329>
> >
> >     Why isn't 11:00 also materialized?

refer to previous comment.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26620
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Oct. 2, 2013, 7:23 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 303
> > <https://reviews.apache.org/r/13362/diff/7/?file=360298#file360298line303>
> >
> >     Why isn't 12:00 also materialized?
> 
> Bowen Zhang wrote:
>     Because the job ends at that time. Similarly, if you have a job starts at 10am and ends at 11am with frequency of ${coord:minutes(10)}, there will be no materialization at 11:00am.

You are right. Initially thought it was inclusive of start and end. 


> On Oct. 2, 2013, 7:23 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 593
> > <https://reviews.apache.org/r/13362/diff/7/?file=360298#file360298line593>
> >
> >     Can we pass a higher limit (1000 or Integer.MAX_VALUE) here instead of number as it limits the result to number. If it materializes more actions than expected this test will never catch it.

If you could just make this minor change I am good to go. You need not upload this change again in review board. Can directly upload in jira. Will +1 and commit.


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26620
-----------------------------------------------------------


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26620
-----------------------------------------------------------



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51864>

    Why isn't 12:00 also materialized?



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51865>

    Why isn't 11:00 also materialized?



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment51861>

    Can we pass a higher limit (1000 or Integer.MAX_VALUE) here instead of number as it limits the result to number. If it materializes more actions than expected this test will never catch it.


- Rohini Palaniswamy


On Oct. 2, 2013, 12:38 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:38 a.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/command/coord/CoordCommandUtils.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Oct. 2, 2013, 12:38 a.m.)


Review request for oozie, Robert Kanter and Rohini Palaniswamy.


Repository: oozie


Description
-------

oozie-1306 cron scheduling for coordinator job


Diffs (updated)
-----

  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1528286 
  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1528286 
  /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1528286 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1528286 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1528286 
  /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 

Diff: https://reviews.apache.org/r/13362/diff/


Testing
-------


Thanks,

Bowen Zhang


Re: Review Request 13362: oozie-1306

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review26504
-----------------------------------------------------------

Ship it!


LGTM +1

- Robert Kanter


On Sept. 25, 2013, 2:01 a.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2013, 2:01 a.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/command/coord/CoordCommandUtils.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1526083 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1526083 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Sept. 25, 2013, 2:01 a.m.)


Review request for oozie, Robert Kanter and Rohini Palaniswamy.


Changes
-------

DST test cases added


Repository: oozie


Description
-------

oozie-1306 cron scheduling for coordinator job


Diffs (updated)
-----

  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1526083 
  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1526083 
  /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1526083 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1526083 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1526083 
  /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 

Diff: https://reviews.apache.org/r/13362/diff/


Testing
-------


Thanks,

Bowen Zhang


Re: Review Request 13362: oozie-1306

Posted by Robert Kanter <rk...@cloudera.com>.

> On Sept. 4, 2013, 8:54 p.m., Robert Kanter wrote:
> > Did you add any tests with DST?  DST can get really tricky so its important that we test it.  
> > 
> > Also, OOZIE-1449 is, among other things, consolidating the JPA executor classes so we don't have so many of them; you should probably coordinate with Ryota to make sure your patches don't conflict with the JPA stuff.
> 
> Bowen Zhang wrote:
>     So, my question is since we are writing cron syntax in UTC time/oozie processing timezone, then what's the problem with DST since UTC doesn't have DST? If we really want to test DST, we need to create a public function in DateUtil to set oozie processing timezone to PST only for testing purpose. Is this what we are going to do?
>     For JPA executor, my patch doesn't touch JPA executors at all.
> 
> Rohini Palaniswamy wrote:
>     Even though time is in UTC, DST transitions will have an effect if user uses a different timezone in the coordinator.
>     
>     http://oozie.apache.org/docs/3.1.3-incubating/CoordinatorFunctionalSpec.html
>     While Oozie coordinator engine works in UTC, it provides DST support for coordinator applications.
>     
>     And please do add/update test cases that validate nominal time for coord actions materialized for regular (not cron) frequencies also, because the code change impacts that too. Emphasizing heavily on unit tests because the existing tests do not fully cover materialization. They only check for number of actions materialized and not their actual nominal times. If the materialization is wrong, then it will be a big issue.

For the JPA executor, I just meant that in the patch, there are places where you execute a JPA command.  The way this is done will change with OOZIE-1499, for example:
-        jpaService.execute(new CoordActionUpdateForInputCheckJPAExecutor(action));
+        CoordActionQueryExecutor.getInstance().executeUpdate(CoordActionQuery.UPDATE_COORD_ACTION_FOR_INPUTCHECK, action);
So you'll need to make minor changes if your patch goes in after OOZIE-1499 or Ryota will need to make some changes to update this patches use of jpaService.execute(...) if it goes in before OOZIE-1499.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review25903
-----------------------------------------------------------


On Aug. 30, 2013, 10:54 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 10:54 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/command/coord/CoordCommandUtils.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Sept. 4, 2013, 8:54 p.m., Robert Kanter wrote:
> > Did you add any tests with DST?  DST can get really tricky so its important that we test it.  
> > 
> > Also, OOZIE-1449 is, among other things, consolidating the JPA executor classes so we don't have so many of them; you should probably coordinate with Ryota to make sure your patches don't conflict with the JPA stuff.
> 
> Bowen Zhang wrote:
>     So, my question is since we are writing cron syntax in UTC time/oozie processing timezone, then what's the problem with DST since UTC doesn't have DST? If we really want to test DST, we need to create a public function in DateUtil to set oozie processing timezone to PST only for testing purpose. Is this what we are going to do?
>     For JPA executor, my patch doesn't touch JPA executors at all.

Even though time is in UTC, DST transitions will have an effect if user uses a different timezone in the coordinator.

http://oozie.apache.org/docs/3.1.3-incubating/CoordinatorFunctionalSpec.html
While Oozie coordinator engine works in UTC, it provides DST support for coordinator applications.

And please do add/update test cases that validate nominal time for coord actions materialized for regular (not cron) frequencies also, because the code change impacts that too. Emphasizing heavily on unit tests because the existing tests do not fully cover materialization. They only check for number of actions materialized and not their actual nominal times. If the materialization is wrong, then it will be a big issue.


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review25903
-----------------------------------------------------------


On Aug. 30, 2013, 10:54 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 10:54 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/command/coord/CoordCommandUtils.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Sept. 4, 2013, 8:54 p.m., Robert Kanter wrote:
> > Did you add any tests with DST?  DST can get really tricky so its important that we test it.  
> > 
> > Also, OOZIE-1449 is, among other things, consolidating the JPA executor classes so we don't have so many of them; you should probably coordinate with Ryota to make sure your patches don't conflict with the JPA stuff.

So, my question is since we are writing cron syntax in UTC time/oozie processing timezone, then what's the problem with DST since UTC doesn't have DST? If we really want to test DST, we need to create a public function in DateUtil to set oozie processing timezone to PST only for testing purpose. Is this what we are going to do?
For JPA executor, my patch doesn't touch JPA executors at all.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review25903
-----------------------------------------------------------


On Aug. 30, 2013, 10:54 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 10:54 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/command/coord/CoordCommandUtils.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review25903
-----------------------------------------------------------


Did you add any tests with DST?  DST can get really tricky so its important that we test it.  

Also, OOZIE-1449 is, among other things, consolidating the JPA executor classes so we don't have so many of them; you should probably coordinate with Ryota to make sure your patches don't conflict with the JPA stuff.  

- Robert Kanter


On Aug. 30, 2013, 10:54 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 10:54 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/command/coord/CoordCommandUtils.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
>   /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
>   /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13362/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bowen Zhang
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Aug. 30, 2013, 10:54 p.m.)


Review request for oozie, Robert Kanter and Rohini Palaniswamy.


Repository: oozie


Description
-------

oozie-1306 cron scheduling for coordinator job


Diffs (updated)
-----

  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1518847 
  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
  /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
  /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 

Diff: https://reviews.apache.org/r/13362/diff/


Testing
-------


Thanks,

Bowen Zhang


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Aug. 29, 2013, 10:52 p.m.)


Review request for oozie, Robert Kanter and Rohini Palaniswamy.


Repository: oozie


Description
-------

oozie-1306 cron scheduling for coordinator job


Diffs (updated)
-----

  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1518847 
  /trunk/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1518847 
  /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java 1518847 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 1518847 
  /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1518847 
  /trunk/examples/src/main/apps/cron-schedule/coordinator.xml PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/job.properties PRE-CREATION 
  /trunk/examples/src/main/apps/cron-schedule/workflow.xml PRE-CREATION 

Diff: https://reviews.apache.org/r/13362/diff/


Testing
-------


Thanks,

Bowen Zhang


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Aug. 9, 2013, 5:45 p.m., Bowen Zhang wrote:
> > /trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java, lines 721-722
> > <https://reviews.apache.org/r/13362/diff/3/?file=338697#file338697line721>
> >
> >     No, in the past with constant frequency, we don't need this statement since a coordinator job is guaranteed to finish running after the last action is done. With cron frequency, you can have a coordinator job that finishes last action while still have more materializations to go even though it won't create any actions. When this coord job reaches end time, its last action is long over.
> >     Take this job as an example: freq="0,30 1-2 * * *", startTime=2013-08-11T01:00Z, endtime=2013-08-11T04:00Z. Right after 2:30, the status transit service won't update the terminal status of this job since it's not done with materialization although its last action is finished according to the frequency.

We should be setting the done materialization to true at 2:30, as there are no more actions to materialize, instead of adding this check to StatusTransitService. The current approach also would mark the coord job SUCCEEDED 1.5 hrs after it actually succeeded which is not what a user would expect. 


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24928
-----------------------------------------------------------


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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24928
-----------------------------------------------------------



/trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/13362/#comment49073>

    No, in the past with constant frequency, we don't need this statement since a coordinator job is guaranteed to finish running after the last action is done. With cron frequency, you can have a coordinator job that finishes last action while still have more materializations to go even though it won't create any actions. When this coord job reaches end time, its last action is long over.
    Take this job as an example: freq="0,30 1-2 * * *", startTime=2013-08-11T01:00Z, endtime=2013-08-11T04:00Z. Right after 2:30, the status transit service won't update the terminal status of this job since it's not done with materialization although its last action is finished according to the frequency. 


- Bowen Zhang


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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Aug. 9, 2013, 4:56 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 116
> > <https://reviews.apache.org/r/13362/diff/3/?file=338699#file338699line116>
> >
> >     Can we have some test cases for DST transitions?
> 
> Bowen Zhang wrote:
>     Can you be more specific about DST transition? what do you want to test? We are all using UTC right here.
> 
> Rohini Palaniswamy wrote:
>     Look at TestCoordELFunctions.testCurrent for Spring and Winter DST transition cases. Adding/Subtracting the frequency interval every time in the loop instead of multiplying like it is done right now gave different results. Refer to the comments in CoordELFunctions.coord_currentRange_sync. Since you are trying to do the same thing in getNextActionMeasureTime, you might have wrong materializations during DST and currently there are no test cases to catch it.

modified the way we do getNextActionMeasureTime.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24923
-----------------------------------------------------------


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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On Aug. 9, 2013, 4:56 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 116
> > <https://reviews.apache.org/r/13362/diff/3/?file=338699#file338699line116>
> >
> >     Can we have some test cases for DST transitions?
> 
> Bowen Zhang wrote:
>     Can you be more specific about DST transition? what do you want to test? We are all using UTC right here.

Look at TestCoordELFunctions.testCurrent for Spring and Winter DST transition cases. Adding/Subtracting the frequency interval every time in the loop instead of multiplying like it is done right now gave different results. Refer to the comments in CoordELFunctions.coord_currentRange_sync. Since you are trying to do the same thing in getNextActionMeasureTime, you might have wrong materializations during DST and currently there are no test cases to catch it.


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24923
-----------------------------------------------------------


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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

> On Aug. 9, 2013, 4:56 p.m., Rohini Palaniswamy wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 116
> > <https://reviews.apache.org/r/13362/diff/3/?file=338699#file338699line116>
> >
> >     Can we have some test cases for DST transitions?

Can you be more specific about DST transition? what do you want to test? We are all using UTC right here.


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24923
-----------------------------------------------------------


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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24923
-----------------------------------------------------------


Still stepping through and trying to verify the logic in coord materialize command. Will update any comments from those soon. 


/trunk/core/src/main/java/org/apache/oozie/service/StatusTransitService.java
<https://reviews.apache.org/r/13362/#comment49067>

    For what scenario do you need this condition specially? Won't CoordActionsGetByLastModifiedTimeJPAExecutor already handle this?  



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment49070>

    Can we have some test cases for DST transitions?



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment49068>

    Can you actually verify the nominal time of the actions created instead of the count and status? That will actually validate if the materialization was done right. 



/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobsGetRunningPastEndtimeJPAExecutor.java
<https://reviews.apache.org/r/13362/#comment49069>

    Can you please remove the not null checks? If they are null, following asserts will anyways fail.


- 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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
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 (updated)
-----

  /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


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.

On Aug. 7, 2013, 5:12 p.m., Bowen Zhang wrote:
> > Have you done any manual testing? (you should be able to play with the computer clock to "trick" Oozie)

We did a lot of manual testing in the past several weeks


- Bowen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24807
-----------------------------------------------------------


On Aug. 6, 2013, 11:36 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 11:36 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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Robert Kanter <rk...@cloudera.com>.

On Aug. 7, 2013, 5:12 p.m., Bowen Zhang wrote:
> > Have you done any manual testing? (you should be able to play with the computer clock to "trick" Oozie)
> 
> Bowen Zhang wrote:
>     We did a lot of manual testing in the past several weeks

great!


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24807
-----------------------------------------------------------


On Aug. 6, 2013, 11:36 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 11:36 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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/#review24807
-----------------------------------------------------------



/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
<https://reviews.apache.org/r/13362/#comment48932>

    These are not used until line 686/687 and can be declared in that else block instead of up here



/trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobsGetRunningPastEndtimeJPAExecutor.java
<https://reviews.apache.org/r/13362/#comment48931>

    List<String> execute



/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
<https://reviews.apache.org/r/13362/#comment48930>

    Unused Import



/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobsGetRunningPastEndtimeJPAExecutor.java
<https://reviews.apache.org/r/13362/#comment48928>

    Unused import



/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobsGetRunningPastEndtimeJPAExecutor.java
<https://reviews.apache.org/r/13362/#comment48926>

    Don't need this comment here



/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobsGetRunningPastEndtimeJPAExecutor.java
<https://reviews.apache.org/r/13362/#comment48929>

    Can you add some more tests?  Like some where the job(s) shouldn't returned by the CoordJobsGetRunningPastEndtime JPA command.


Have you done any manual testing? (you should be able to play with the computer clock to "trick" Oozie)

- Robert Kanter


On Aug. 6, 2013, 11:36 p.m., Bowen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13362/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 11:36 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
> 
>


Re: Review Request 13362: oozie-1306

Posted by Bowen Zhang <bo...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13362/
-----------------------------------------------------------

(Updated Aug. 6, 2013, 11:36 p.m.)


Review request for oozie, Robert Kanter and Rohini Palaniswamy.


Changes
-------

Forget to add new files


Repository: oozie


Description
-------

oozie-1306 cron scheduling for coordinator job


Diffs (updated)
-----

  /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