vsivsi / meteor-job-collection

A persistent and reactive job queue for Meteor, supporting distributed workers that can run anywhere.
https://atmospherejs.com/vsivsi/job-collection
Other
388 stars 68 forks source link

Repeating does not work as documented #217

Closed mitar closed 7 years ago

mitar commented 7 years ago

Currently, in documentation, it is written:

schedule -- Repeat using a valid later.js schedule. The first run of this job will occur at the first valid scheduled time unless .after() or .delay() have been called, in which case it will run at the first scheduled time thereafter.

I understand this that if I have a schedule that job should run every 4 hours (midnight, 4 AM, 8 AM and so on), and I call if without after or delay, the first time it will run will be at some of those 4 hours times.

This is not what happens. The first time is run almost immediately, and then the rerun is done at 4 AM, 8 AM and so on.

I think the reason is that scheduling more or less gets converted to this call:

later.schedule(later.parse.text('every 4 hours')).next(1, new Date())

The reason for that is because doc.after is set to default new Date(0), so doc.after gets set to new Date() before calling next(1, doc.after). And now the strange thing. It seems if the second argument is in the past, then next simply returns the current time, and if it is in the future, then it returns the correct time in the future.

Examples from my web console in the browser:

[time = new Date(), later.schedule(later.parse.text('every 4 hours')).next(1, time)]
[Mon Feb 20 2017 00:57:20 GMT-0800 (PST), Mon Feb 20 2017 00:57:20 GMT-0800 (PST)]

[time = new Date(), later.schedule(later.parse.text('every 4 hours')).next(1, new Date(time.valueOf() + 80000))]
[Mon Feb 20 2017 00:58:40 GMT-0800 (PST), Mon Feb 20 2017 04:00:00 GMT-0800 (PST)]
mitar commented 7 years ago

Possible workaround:

later.schedule(later.parse.text('every 4 hours')).next(2, later.schedule(later.parse.text('every 4 hours')).prev(1, time))[1]
Mon Feb 20 2017 04:00:00 GMT-0800 (PST)

So it seems it returns current time if second argument is between noon and 1 AM, and if it is after 1 AM, then it return 4 AM as next time.

mitar commented 7 years ago

It seems this is a known workaround.

I will open a pull request to use this workaround here.

vsivsi commented 7 years ago

Hmmm, this is not what I see with the sample app at https://jcplayground.meteorapp.com/

screen shot 2017-02-21 at 11 50 03 am

vsivsi commented 7 years ago

Nevermind the above, I see now what you are saying (now that it is after noon local time, the above runs immediately and presumably will for the next hour...)

vsivsi commented 7 years ago

Just merged your PR on this.