laravel / horizon

Dashboard and code-driven configuration for Laravel queues.
https://laravel.com/docs/horizon
MIT License
3.83k stars 643 forks source link

Implement 'scaleToMin' configuration option for supervisor startup scaling method #1384

Closed cyppe closed 6 months ago

cyppe commented 6 months ago

This pull request introduces a new configuration option, scaleToMin, as a response to the community discussion in Issue #1295.

The addition of this feature enhances the flexibility and efficiency of Horizon's supervisor scaling behavior, particularly for environments where resource conservation is critical.

Motivation: Currently, Horizon supervisors scale directly to the maximum number of processes (maxProcesses) upon startup. This behavior can lead to unnecessary resource usage (CPU/memory) in scenarios where the workload does not immediately require such capacity. The community has expressed a need for a more resource-efficient approach to scaling, especially important for applications with variable job loads.

Implementation: The scaleToMin option allows developers to specify that supervisors should initially scale to the minimum number of processes (minProcesses, defaulting to 1) and then use the configured auto-scaling strategy to adjust the number of workers based on workload. This approach optimizes resource utilization by starting with a conservative number of worker processes and scaling up only as required.

Configuration: To enable this behavior, set 'scaleToMin' => true in the relevant supervisor configuration within the horizon.php config file. For example:

'supervisor-1' => [
    ...
    'scaleToMin' => true,
    ...
],

When scaleToMin is enabled, initiating Horizon with php artisan horizon will start supervisors with minProcesses instead of maxProcesses.

Benefits:

I look forward to your feedback and hope this contribution proves valuable.

taylorotwell commented 6 months ago

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

marcoskubis commented 6 months ago

It appears to be an automated message.

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

ryzr commented 3 months ago

Hope this is reconsidered in future - starting at maxProcesses makes autoscaling on memory usage quite tricky.

mbardelmeijer commented 2 months ago

Would love to see support for this as well. We're running into same issues were we can support peak load of some queues of maxProcesses, but having all the queues at max processes on start-up gives peak loads.

Perhaps Horizon can be modified to start up on the average value between min & max processes by default?

dbpolito commented 1 month ago

I completely agree that we should start using min instead of max... and i completely feel you guys how frustrating it is sometimes when they close the PR / Issue like this.

If feels their OCD for not having opened issues / PRs is more important...

Like this one: https://github.com/laravel/horizon/pull/1442#issuecomment-2127826748

If you have no time, burying it instead of leaving it open for when the time comes feels pretty bad and waste of the time spent....

I ended up overwriting the SupervisorFactory and Supervisor in my project and overwrote the scale method with:

    public function scale($processes)
    {
        if (debug_backtrace()[1]['class'] === SupervisorCommand::class) {
            return;
        }

        parent::scale($processes);
    }

To ignore this call: https://github.com/laravel/horizon/blob/5.x/src/Console/SupervisorCommand.php#L99-L101