-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
perf(cron): Delay (re)checking timed jobs #50768
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
Conversation
|
Tests might be nice for this, and it's not hard, the test infra for bg jobs is quite nice, imho: https://github.com/nextcloud/server/blob/master/tests/lib/BackgroundJob/JobListTest.php |
| // re-checking with every cron run. | ||
| // To avoid bugs that lead to jobs never executing again, the future timestamp is | ||
| // capped at two days. | ||
| $nextCheck = min($nextPossibleRun, $now + 48 * 3600); |
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 have more details on why the 48 hours?
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.
I was thinking that someone might set a (dynamic) interval that is too high so that a job is never checked again. This gives the job a chance to recover.
For any jobs that run rarely (like every two weeks) and disabled apps, that would also help clear up the row earlier.
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.
Is it not possible to add a condition on lastrun+interval as part of the SQL query instead?
3fa3611 to
8bc9498
Compare
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
8bc9498 to
2395526
Compare
|
/backport to stable31 |

Summary
Right now timed jobs are always loaded from the db and instantiated. Jobs with intervals higher than the cron interval are continuously loaded and checked,
\OCP\BackgroundJob\TimedJob::startprevents the actual execution.As an optimization we can preemptively check if a job is ready. Otherwise we park it for a later check.
How to test
TODO
Checklist