-
Notifications
You must be signed in to change notification settings - Fork 800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor JOB_PROCESS_WHERE_QUERY to get an optimized query plan #869
Refactor JOB_PROCESS_WHERE_QUERY to get an optimized query plan #869
Conversation
disabled: {$ne: true} | ||
}, { | ||
name: jobName, | ||
lockedAt: {$exists: false}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if this lockedAt: {$exists: false}
should be one of the options in $or
?
I'm just a little confused because I'm seeing only lockedAt: {$eq: null}
and lockedAt: {$lte: lockDeadline}
from the old query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK (from the top of my head) your can check both existence and null equality with "$eq: null". Whereas the "$exists: false" checks the existence only.
But I don't trust my memory too much. 🙂 So, please double check.
In other words, the PR is okay IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has caused a migration bug from v2->v4.
Here are the details: #1238
Thank you! I appreciate elaborate testing instructions. Just a small questi on before we could merge this; #869 (review) Pardon that it took so long to get back to this. :-) |
This pull request is a simple "refactoring" of the job-pulling query performed by agenda, that yieldsbetter results when there are lots of "expired" jobs in database. As far as I can see, there's no functional impact.
Context:
I recently ended up in a situation with a lot of "expired" jobs in database, i.e. jobs with
nextRunAt
< current date.In that situation, I saw mongod CPU consumption climbing pretty high, and after checking mongo logs, I saw that the regular "job pulling" request performed by agenda scanned a lot of index keys.
I managed to reproduce this on my computer. I first created 100k "expired Jobs":
Then checked the explain plan of the job pulling request done by agenda:
So 10k index keys were examined :(
One of my colleague played with the query, and found out that with a simple "refactoring" of the request, we got a much better query plan:
Only 100~ keys examined!