Set default "after" field to a past date

[bratchenko]

If we set it to new Date() and for some reason client's unixtime is ahead of server's unixtime, job will wait in queue for no reason.

[vsivsi]

Look good. Thanks for catching this. Gotta love a 1 character pull request! :-)

[vsivsi]

@daeq Hi, I just wanted to let you know that I've decided to revert back to the previous behavior for this.

My reasoning is this: the default after value in the job document provides useful information (such as to easily sort on it for displaying jobs in a UI). With the change in the PR, all jobs that don't specifically call job.after() will sort to the same position (45 years ago).

I understand the problem you were trying to solve, but it has an extremely simple solution:

job.after(new Date(timeInPast));

You can call job.after() yourself and provide whatever value you would like, including dates as far back into the past as you would like.

IMO, losing the (by default) sortable information in the job after field isn't worth the (very small) gain in convenience of not having to explicitly call after() to solve the clock skew case you highlighted.

Thanks again for the PR, and for making me aware of this potential issue of unsynchronized clocks.

[bratchenko]

Hi!

job.after(new Date(timeInPast));

Yes, this is a solution I use now. But it looks hacky. And I need to not forged to use it each time I create a job since there are no circumstances in which I'd like new job to be executed not immediately by default.

losing the (by default) sortable information in the job after field isn't worth the (very small) gain in convenience of not having to explicitly call after() to solve the clock skew case you highlighted.

I think it's reasonable to add additional field like "createdTime" for this purpose.

There isn't much sense in making job to run after specific time by default. I'd personally would prefer this field (after) to be nullable and if it isn't set, job just runs immediately on the next poll.

[vsivsi]

If I understand correctly, when the machine creating the job has a RTC that is a little ahead of the server that is promoting jobs from waiting--> ready, then the job will (hopefully) only wait one more promote cycle before becoming ready. As it is, all jobs enter the collection in the waiting state, and on average wait 1/2 of the promotion interval to become ready. So you have some jobs waiting, on average 1.5 promotion cycles to be ready?

Or is your issue that you have (untrusted) client browsers creating jobs, and you don't control those clocks? If that's the case, I'd write a server method to create the job, and take the client's clock out of the picture entirely, since its clock could be set to anything.

I think it's reasonable to add additional field like "createdTime" for this purpose.

The creation time is already recorded, but it's in the log array, which makes it difficult to use as a sort key. In any case, after is really what I want to sort by, so that failed jobs that are retrying show up in order of the time of the next attempt, not when they were originally created.

I need to think about this a bit but perhaps the right answer is for the default after value to be null before job.save(), and have after be set on the server in that case. That would solve your clock skew case for "run immediately" as the server clock would become the source of truth.

[bratchenko]

I like your thoughts. The solution from the last paragraph looks like ideal to me.

Don't like the idea of writing additional code to create jobs on server. The idea of meteor is that there should be as little difference as possible between client and server code.

[vsivsi]

I've just implemented a solution that's halfway between the two...

On the machine creating a job, job.after() defaults to new Date(0) just as you suggested the PR. On the server, when job.save() happens, if the value of after is in the past (relative to the server clock) then it is updated to the current server time.

So you get the behavior you want and the meaning and use of after in all of the internal queries is preserved. For example, when jc.getWork() is run, the server returns the highest priority job first, but if there are multiple jobs at that same priority, then it returns the job with the earliest value of after, which is of benefit even for "run immediately" type jobs, since the work becomes FIFO ordered for jobs of equal priority.

I'll try to release this tomorrow.

[vsivsi]

If you'd like to try it out, it's on the add_until branch: https://github.com/vsivsi/meteor-job-collection/tree/add_until

[vsivsi]

@daeq I should warn you, that branch contains upcoming features that change the job document model, so don't run it against a database you care about!

[bratchenko]

Sounds like good solution.

The only problem would be if servers that run/store jobs have different unixtime. But it's much more rare case and is error by itself.

[vsivsi]

Yes, the server RTC is pretty critical since it is also being used to determine when jobs are eligible to run. If you have multiple Meteor servers behind a load balancer and their clocks are off, then you probably have much bigger problems!

[vsivsi]

@daeq This is live on Atmosphere in jobCollection v0.0.16. Thanks again for filing this issue.