roots / trellis

WordPress LEMP stack with PHP 8.2, Composer, WP-CLI and more
https://roots.io/trellis/
MIT License
2.49k stars 609 forks source link

Check if cron not running already when triggering another cron #1178

Open maciekpaprocki opened 4 years ago

maciekpaprocki commented 4 years ago

Summary

Make sure that cron is not running already when triggering another cron.

Motivation

The other day I found a peculiar bug in one of the plugins on my website. cron option in DB (wp_options name='cron') had thousands of duplicates of one cron job scheduled. That caused cron to run for hours. When few crons overlapped that made the server run out of resources. Obviously, the solution is to clear the scheduled jobs and fix the plugin, but I think there would be quite a lot of value in making cron check if the previous cron finished running before starting another one ( or even allow certain amount of parallel runs you can adjust yourself ).

I am happy to work on that if other people see the value in it.

swalkinshaw commented 4 years ago

I'd be interested in a potential solution but I'm a bit wary of adding that complexity by default. Mostly since I don't know what the solution would look like 🤷‍♂️

maciekpaprocki commented 4 years ago

It would be probably something like: from: cd path_to_wp && wp cron event run --due-now > /dev/null 2>&1

to pgrep 'wp cron event run --path=${path_to_wp}' || wp cron event run -path=${path_to_wp} > /dev/null 2>&1

It would have to be limited to domain and to do that path would have to be one of the params of function.

Let me know if that's fine. If yes, I will send a PR.

jordan26 commented 4 years ago

I have this same issue. Our cron runs for ages and the next one often starts before the first had finished, causing an overlap.

Introducing a cron lock file may be a solution. https://ma.ttias.be/prevent-cronjobs-from-overlapping-in-linux/

Note, if using something like a lock file, be sure to check if a cron fails or crashes, the lock file is still deleted eventually else the next cron will never run.

Use another cron to check the date/time of the existing lock file, if older than X delete it. Just an idea. I don't have enough experience to build this yet, but I'll try.

+1 to this original request.

swalkinshaw commented 2 years ago

Sorry just getting to this. @maciekpaprocki would you still be interesting in submitting a PR?

It would have to be limited to domain and to do that path would have to be one of the params of function.

By "domain", you mean the path to the site right? eg: /srv/www/mysite.com

maciekpaprocki commented 2 years ago

Hey, generally something around solution I gave should work. Maybe some logging added also when that happens. Lock would be maybe nicer, but I am not sure if it gives it much.

I might maybe take a look on it one day, but I am not gonna lie, don't have much spare time for open source recently.