It seems like the latest (most recent) is the one that runs first, as opposed to being the first one queued for the same job name.
@niftylettuce @lushc do you recon https://github.com/agenda/agenda/pull/451 would fix this?
Hmm, I don't think #451 will fix this one unfortunately, seems to be related to the in-memory queue, looks like it is indeed treated as LIFO when processing jobs to run:
// Get the next job that is not blocked by concurrency
var next;
for(next = jobQueue.length - 1; next > 0; next -= 1) {
var def = definitions[jobQueue[next].attrs.name];
if(def.concurrency > def.running) break;
}
var job = jobQueue.splice(next, 1)[0];
This was implemented in commits "Processing the next job that is not blocked by concurrency" and "Requeuing concurrency blocked jobs wrt priority". It seems the LIFO behaviour is integral to ensuring jobs previously blocked by concurrency limits can run ASAP, and also sorting which jobs to run based on their priority.
So yeah I'm not sure if we could guarantee the running order based on when the jobs were queued anyway due to the dynamic re-prioritising. If you're queuing X number of jobs at once and want to control their order of execution I guess you could use priority with a decaying value, e.g.
const jobs = [];
// batch up new jobs to save
jobs.push(agenda.create('myJob', {data: true}).schedule('in 30 seconds'));
// ...
// earlier jobs will have higher priority
let priority = jobs.length;
for (let job of jobs) {
job.priority(priority--).save(); // async!
}
Does this help?
It unfortunately does not @simison (thanks though - at least for my use case it doesn't.
I would think a standard behavior should be run in the order queued - I think this is a core bug IMO!
Hope we can get this fixed and that others agree 馃憤
Appreciate all your help here,
@niftylettuce does that code bit @lushc provided help?
To me it seems like adding that to docs and mentioning we can't ensure running order would be good improvement at this point.
Suprised to know about the LIFO behavior. This essentially means that Agenda can't be used as a task queue ? Very fundamental Bug IMO.
@niftylettuce if we could get some more details surrounding your use-case it'd help clarify the issue. As far as I'm aware it's the first time something like this has come up, so anything to document why this isn't desirable behaviour will be useful.
@gashokvr I believe Agenda was primarily developed to be a job scheduler, guaranteeing the execution of a task at a repeated interval, so the focus has been on when to run a job rather than the order in which it's run. That said, you can still prioritise jobs with priority() and control running order that way. Any info you can give on how the LIFO behaviour isn't working for you is appreciated.
And of course, a passing PR to change it to FIFO would be most welcome :thumbsup:
@simison I've submitted a pull request #464 to lib/agenda.js. Basically I modify the findOneAndUpdate function options - we can use _id as a sorting value; it's a timestamp - via ObjectID.getTimestamp. Since MongoDB sorts in order from left to right based off query, it will sort by timestamp first (ascending - so oldest first; obeys first in first out) and then priority (descending - so highest priority jobs first)
I've also documented this in the Readme as well in the pull request:
Jobs are run with priority in a first in first out order (so they will be run in the order they were queued/created _AND_ with respect to highest priority). We utilize MongoDB's ObjectID through the job record property of _id as a sorting value (since it is a timestamp in itself).
For example, if we have two jobs named "send-email" queued (both with the same priority), and the first job is queued at 3:00 PM and second job is queued at 3:05 PM with the same priority value, then the first job will run first if we start to send "send-email" jobs at 3:10 PM. However if the first job has a priority of 5 and the second job has a priority of 10, then the second will run first (priority takes precedence) at 3:10 PM.
@lushc My use case is simply the example I highlighted in my updates to the Readme in my prior comment. Another use case is when { concurrency: 1 } is set to 1, so only one job at a time can be run. There could be 10 jobs, all with the same priority, and I want the first to go out at once. A simple example of this is that if you queue emails, and have an email that says a "Job Started" and another email that says a "Job Finished" - so you obviously want the "Job Started" email to be delivered first.
Also submitted a PR 馃憤
@niftylettuce would you mind adding a way to change that sort? For example my application could change the sort to use the plan of my users to have the top tier plans get first priority without needing to mess with the job's priority.
added @OmgImAlexis - you can specify config.sort now - the default is { _id: 1, priority: -1 }.
Can we please get this merged quickly? Thank you all so much.
@OmgImAlexis need me to take those eslint commits out or something? just lmk! thank you all! cc @simison
Yes can you please take out the eslint stuff and rebase.
@OmgImAlexis Welp that was annoying... didn't realize you had merged/pushed to master... I'm way too tired but wanted to get this in for you. Need sleep. Plz merge, thanks
Rebased and should be good to merge now cleanly
@OmgImAlexis I'm adding tests now, can you or @simison please accept this PR once tests are added and merge + release new version? Thank you - I should have it done shortly.
@OmgImAlexis so I've added tests (and glad I did) - interestingly... the sort query { _id: 1, priority: -1 } is not working properly, however { priority: -1 } does work and keep them in order. I'm wondering if this is a bug with MongoDB. Trying to figure this out now.
OK tests are added and pass, should be good to go. Please review and accept the PR in #464, and release new version to NPM and as a tag on GitHub - then you can close the following issues: #462 and #440
Great, thanks for adding tests!
@niftylettuce @OmgImAlexis am I correct that this change is calling for a major semver bump? (1.x.x)
I'd suggest to do a major version bump because people might unknowingly be relying on the old sort method @simison as an unintended feature haha.
This PR should be good to go now, see #464
Most helpful comment
added @OmgImAlexis - you can specify
config.sortnow - the default is{ _id: 1, priority: -1 }.