Skip to content
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

[11.x] Add sometimeDaily() to scheduler #52331

Closed
wants to merge 1 commit into from

Conversation

inxilpro
Copy link
Contributor

Often times you have scheduled commands that you want to run every day, but you don't particularly care when during the day they run. For apps that have many scheduled tasks, this can unintentionally cause a huge spike in command execution every midnight—the default for daily().

This PR introduces a sometimeDaily() method that incrementally schedules tasks 5 minutes apart from each other throughout the day:

$schedule->command('model:prune')->sometimeDaily();       // scheduled at 00:05
$schedule->command('telescope:prune')->sometimeDaily();   // scheduled at 00:10
$schedule->command('cloudflare:reload')->sometimeDaily(); // scheduled at 00:15

If, somehow, you manage to fill out every 5-minute increment, it will reset back to midnight and start again.

@devajmeireles
Copy link
Contributor

I don't know if what I'm thinking makes sense, but for me, it would be more appropriate to call this sometimeMinutes, the term daily gives the impression that it will occur daily, just once, like the original proposal for daily

@inxilpro
Copy link
Contributor Author

the term daily gives the impression that it will occur daily, just once

It will happen once daily. Each call is essentially a macro for ->dailyAt() that changes the hour and minute passed every call.

@gabrielrbarbosa
Copy link

gabrielrbarbosa commented Jul 30, 2024

Personally, I prefer closures with dailyAt method like this

Schedule::call(function () {
    Schedule::command('refresh-integration-token');
    Schedule::command('telescope:prune --hours=168');
    Schedule::command('cache:clear');
})->dailyAt('00:00');

Schedule::call(function () {
    DB::table('activity_log')->where('created_at', '<', now()->subYear())->delete();
    Storage::disk('local')->delete(Storage::disk('local')->allFiles());
})->dailyAt('02:00');

Schedule::call(function () {
    Schedule::command('send:birthday-messages');
    Schedule::command('send:order-alerts');
})->dailyAt('07:00');

@x7ryan
Copy link

x7ryan commented Jul 30, 2024

I like it, my only suggestion would be maybe an optional parameter to customize how far apart they are scheduled in case you have longer running tasks or tasks you want to space out more for some other reason.

@ClaraLeigh
Copy link

This is how I wished the normal daily worked. Definitely fits a need I have

The only project I wouldn't use it on is one that has a peak usage time. Makes me wish there was a dailyBetween() along the same lines

@inxilpro
Copy link
Contributor Author

Makes me wish there was a dailyBetween() along the same lines

Ooh, I do like that! It'd be nice to schedule all these "cleanup" style tasks during slow hours only.

I might take a crack at tweaking this tomorrow!

@khalyomede
Copy link
Contributor

I like the idea, I would have imagined something that would balance out the commands during the day evenly depending on the number of commands that want to be balanced. May I suggest something like this?

Schedule::command('artisan:inspire')->duringTheDay();
Schedule::command('artisan:info')->duringTheDay();

Schedule::command('migrate')->duringTheHour();
Schedule::command('db:seed')->duringTheHour();
Schedule::command('db:prune')->duringTheHour();

Which would:

  • Schedule artisan:inspire every day at 00:00
  • schedule artisan:info every day at 12:00
  • Schedule migrate every hours at minute 00
  • Schedule db:seed every hour at minute 20
  • Schedule db:prune every hour at minute 40

WDYTAI?

@taylorotwell
Copy link
Member

taylorotwell commented Aug 1, 2024

I wonder if this would be a bit better like:

Schedule::call(fn)->hourly()->nextVacancy();
Schedule::call(fn)->daily()->nextVacancy();
Schedule::call(fn)->weekly()->nextVacancy();
Schedule::call(fn)->quarterly()->nextVacancy(after: 5); // 5 minutes after next quarterly scheduled task

@inxilpro
Copy link
Contributor Author

inxilpro commented Aug 2, 2024

@taylorotwell I shied away from something like that because it would involve tracking more data across the scheduler. If you're OK with a bigger refactor, I'm up for taking a stab at it. The way I see it, for something like that to work we'd either need to make the existing schedule available to future tasks, or track a bunch of data about the last/next scheduled task of each type.

@taylorotwell
Copy link
Member

I dunno - I'm close to thinking this is a neat idea but I can't decide if it's really going to be that trustworthy in practice. The 5 minute time feels a bit arbitrary and isn't configurable and doesn't help you at all if most of your commands take longer than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants