search

spatie / laravel-short-schedule

Schedule artisan commands to run at a sub-minute frequency
https://freek.dev/1683-a-package-to-schedule-artisan-commands-at-sub-minute-frequencies
MIT License
611 stars 52 forks source link

Mitigate memory leaks #34

Closed Jakhongir91 closed 3 years ago

Jakhongir91 commented 3 years ago

Closes issue #17

What has been done (and why) A lifetime of short schedule's worker has been added to mitigate RAM leaks when running frequent processes every seconds with allowing overlapping.

How does it work A new property $lifetime was added in Spatie\ShortSchedule\ShortSchedule class. In ShortSchedule class in run method, after adding every scheduled task to loop it will check if the lifetime property is set and if it was, it will add event loop termination. The default lifetime for production environment is 30 seconds For dev mode there is not lifetime constraint, to make it possible to run php artisan short-schedule:run command in console without auto exiting, when developing and testing features locally.

How to test

  1. Set 5 short scheduled tasks in App\Console\Kernel.php with frequency of 1 second, don't use withoutOverlapping() method.

  2. When running in non production mode (APP_ENV=local in .env file), execute in console php artisan short-schedule:run --lifetime=5. After each task will be executed once, the worker will terminate itself and all memory will be dismissed at once.

  3. When running in production mode (APP_ENV=production in .env file) configure supervisor to run php artisan short-schedule:run. Then the worker will terminate itself each 5 seconds and supervisor will bring it back.

Tests At the moment I do not know how to create autotests for this PR

freekmurze commented 3 years ago

Thank you!

bushbill commented 3 years ago

Thank you for this option, but the default value of 30 is not optimal. Better to have the standard behavior, if the parameter is not set: "run forever"!

And a note in the README about the 30 seconds in production is missing!

freekmurze commented 3 years ago

@bushbill I'll remove the bit that sets the default in production.