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
385 stars 68 forks source link

job.after() only runs after first available time? #162

Closed JulianKingman closed 8 years ago

JulianKingman commented 8 years ago

I can't seem to get a job to start right away. Here's what I've tried:

job.after(new Date());
//sets it to same time as now, but tomorrow
job.after(moment().add(-1, 'days').toDate());
//should be yesterday, same result as above
job.after(moment().add(2, 'days').toDate());
//future dates like this work fine

Not putting .after() seems to set after to tomorrow at today's time, it's as if it can only run on the next instance (I'm using every 1 day). Am I doing something wrong here?

vsivsi commented 8 years ago

Just so I understand, you are using something along the lines of:

job.repeat({
   // run this job now and then every day at 0:00 UTC
   schedule: jc.later.parse.text('every 1 day'); 
});

I just tried that in the playground app at https://jcplayground.meteor.com/ and it works just fine. You can see in the source that it doesn't use job.after() at all. https://github.com/vsivsi/meteor-job-collection-playground/blob/master/play.coffee#L283

It may be that you're running up against a quirk in later.js occurrences. job-collection uses the later.js later.schedule(...).next(1); call to determine the next time to run. However, it won't run until the first time after any job.delay() or job.after(). Setting these to a value in the past doesn't work because the server resets any past job.after() value to be the current time.

What puzzles me is:

job.after(new Date());
// sets it to same time as now, but tomorrow

That absolutely should not be the case. Are you sure your client and server clocks are correct?! The value of job.doc.after is calculated on the machine that creates the job object, but it is actually used on the server to determine when the job should be made ready to run.

The behavior you are describing is exactly consistent with the job being created on a client with a clock set to tomorrow, but then submitted to a server with a correct clock... (We did just have a leap day!) You can reproduce this by setting your computer's clock to tomorrow, and then submitting a new job on https://jcplayground.meteor.com. It won't run until tomorrow.

JulianKingman commented 8 years ago

I'm running this on localhost, so only one clock :) That later.js quirk sounds about right, though. What I'm trying to do here is allow someone to start a job 'today', effectively on today's date at 0:00. Now that I look at it, the new Date() option actually sets it to the same time as what the next occurrence should be. So if the next occurrence should be 5pm today (it's now 3/9/16 at 4:34pm), 'after' shows up as 3/10/16 at 5pm, not 3/10/16 at 4:34pm as I suggested in my first post. So, is the only solution here to not use after() at all for things that need to happen immediately? I would like to just use job.after(startDate), and not have to worry about checking if startDate is today, and if so not running the function. Can it be changed to something like later.schedule(...).next(0)?

vsivsi commented 8 years ago

Hi, if you want the first job to run immediately then there's no need to explicitly use job.after(). If you don't use .after() or .delay() then job.doc.after = new Date() on the server.

Here's what actually happens with job.after() and job.repeat({ schedule: s }):


In the playground app, the effect of this is that for "every 1 day" you will get:

screen shot 2016-03-09 at 3 02 18 pm

And for "every one day at 0:00" you instead get:

screen shot 2016-03-09 at 3 05 33 pm

I hope this explanation helps!

JulianKingman commented 8 years ago

Sorry, I'm really bad at reading coffeescript, I still don't understand why if I set job.after(new Date()) at 4:34pm, with a schedule to run at 5pm, it won't run today at 5pm, but will wait until tomorrow at 5pm.

Anyway, I'm kind of splitting hairs here, since I can see how to work around this, so feel free to close this anytime. At this point I'm just trying to understand it.

Thanks!

vsivsi commented 8 years ago

Hmmm. If that's still actually what's happening then there may be a bug here. Let me investigate and see if I can come up with a failing test.

vsivsi commented 8 years ago

I just modified the playground app to add:

job = new Job(myJobs, myType, { owner: Meteor.userId() })
            .retry({ retries: 2, wait: 30000, backoff: 'exponential'})
            .repeat({ schedule: s })
            .after(new Date())        //   <--- This line is added
            .save({cancelRepeats: true})

And for "every 1 day" there is no change in behavior: a job is scheduled immediately. Moving the .after(new Date()) call to be before the .repeat() also has no effect. Replacing the new Date() with moment().add(1, 'days').toDate() has exactly the expected effect (the job is scheduled for 0:00 UTC tomorrow).

Basically I can't reproduce what you are describing. If you are willing to share a simple app that reproduces what you are describing, I'd be happy to take a look.

JulianKingman commented 8 years ago

OK, I'm getting it. It's the difference between UTC and local time. Is the only way to adjust for that by setting the date to UTC before its put into the job?

vsivsi commented 8 years ago

Later.js only deals in UTC times. So if you say "every 1 day" the repeats will all be at midnight UTC. If you say "every 1 day at 16:00" it will repeat at 16:00 UTC, etc.

All Date objects provided to job -collection (e.g. job.after(date)) are normal JS Dates (local time on the machine where run). They are always converted on the server to EJSON dates for storage and processing. If you would like to specify such dates in UTC or some specific timezone(s), I recommend using the moment.js library to create JS dates for whatever timezone you would like: https://atmospherejs.com/momentjs/moment

JulianKingman commented 8 years ago

I was getting tripped up with the whole UTC time thing. Basically, because I'm ahead or behind of UTC time, the next available time to run (because time is in UTC) is tomorrow (or next available schedule). My solution was pretty much as you suggest, to convert my time to UTC using moment, which looked something like this:

//time is formatted like this: 22:00 (or HH:mm)
var time = '22:00';
//corrected for UTC
time = moment.utc(moment(time,'HH:mm')).format('HH:mm');

Thanks for your help!

vsivsi commented 8 years ago

Glad you figured out what was happening!