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

[8.x] Improve schedule:work command #34736

Merged

Conversation

hivokas
Copy link
Contributor

@hivokas hivokas commented Oct 7, 2020

This pull request fixes the partially incorrect behavior of the schedule:work command (schedule:run is not executed if the previous execution has taken more than 1 minute).

The idea of this PR is to have a pool of executions.

When the execution is started - it's added to the pool.
When the execution is finished - it's removed from the pool.
When the new output from any execution is available - it's outputted.

Let's imagine we have the following scheduler setup:

protected function schedule(Schedule $schedule)
{
    $schedule->command('sleep 1')->everyMinute();
    $schedule->command('sleep 2')->everyMinute();
    $schedule->command('sleep 64')->everyMinute();
    $schedule->command('sleep 128')->everyMinute();
}

sleep command just runs the sleep() function with the specified number of seconds

Currently, the first execution will be performed. However, executions won't be performed during the next few minutes (because executions are performed synchronously).

In this PR executions will be performed asynchronously & you'll see the following output:

Schedule worker started successfully.

Execution #1 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 1 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 2 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 64 > '/dev/null' 2>&1

Execution #2 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 1 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 2 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 64 > '/dev/null' 2>&1

Execution #1 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 128 > '/dev/null' 2>&1

Execution #3 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 1 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 2 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 64 > '/dev/null' 2>&1

Execution #2 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 128 > '/dev/null' 2>&1

Execution #4 output:
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 1 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 2 > '/dev/null' 2>&1
Running scheduled command: '/usr/local/Cellar/php/7.4.10/bin/php' 'artisan' sleep 64 > '/dev/null' 2>&1

What do you think about this idea? Does it look fine?

The code might probably be polished.

Illia Sakovich and others added 2 commits October 7, 2020 23:02
@paras-malhotra
Copy link
Contributor

paras-malhotra commented Oct 8, 2020

Makes sense, that's how the cron would actually run in reality as well. It would spawn a new schedule:run process every minute.

@taylorotwell
Copy link
Member

I added a 100ms sleep in that while loop. Otherwise it's thrashing my CPU.

@taylorotwell taylorotwell merged commit a8d9204 into laravel:8.x Oct 8, 2020
@sikhlana
Copy link
Contributor

sikhlana commented Oct 13, 2020

By a freak of chance if the loop iteration doesn't start at 0 seconds, isn't there a chance the schedule:run command wouldn't be called for that minute?

I also think it would be better to use an async pcntl signal handler to handle SIGTERM and SIGINT.

This is the code I've been using for my company's production environment for a couple of years now without any issues:

/** @var PhpExecutableFinder $finder */
$this->registerSignalHandler();
$executable = $finder->find(false);

do {
    $process = new Process([
        $executable,
        'artisan',
        'schedule:run',
    ], base_path());

    $process->start(function ($type, $data) {
        $this->getOutput()->write($data);
    });

    usleep(60000000);
} while(! $this->quit);

return 0;

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.

5 participants