From ceaaa580ce21b8afa0baca046ee682e83568f294 Mon Sep 17 00:00:00 2001 From: Luke Kuzmish Date: Sat, 4 Mar 2023 07:03:22 -0500 Subject: [PATCH 1/7] adds autoScalingStrategy option --- src/AutoScaler.php | 10 ++++++++-- src/Console/SupervisorCommand.php | 11 ++++++++--- src/QueueCommandString.php | 3 ++- src/SupervisorOptions.php | 24 +++++++++++++++++++++++- tests/Feature/AddSupervisorTest.php | 2 +- tests/Feature/AutoScalerTest.php | 18 ++++++++++++++++++ 6 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/AutoScaler.php b/src/AutoScaler.php index 132f8fb2..825d6321 100644 --- a/src/AutoScaler.php +++ b/src/AutoScaler.php @@ -96,11 +96,17 @@ protected function timeToClearPerQueue(Supervisor $supervisor, Collection $pools protected function numberOfWorkersPerQueue(Supervisor $supervisor, Collection $queues) { $timeToClearAll = $queues->sum('time'); + $totalJobs = $queues->sum('size'); - return $queues->mapWithKeys(function ($timeToClear, $queue) use ($supervisor, $timeToClearAll) { + return $queues->mapWithKeys(function ($timeToClear, $queue) use ($supervisor, $timeToClearAll, $totalJobs) { if ($timeToClearAll > 0 && $supervisor->options->autoScaling()) { - return [$queue => (($timeToClear['time'] / $timeToClearAll) * $supervisor->options->maxProcesses)]; + $numberOfProcesses = $supervisor->options->autoScaleByNumberOfJobs() + ? ($timeToClear['size'] / $totalJobs) + : ($timeToClear['time'] / $timeToClearAll); + $numberOfProcesses *= $supervisor->options->maxProcesses; + + return [$queue => $numberOfProcesses]; } elseif ($timeToClearAll == 0 && $supervisor->options->autoScaling()) { return [ diff --git a/src/Console/SupervisorCommand.php b/src/Console/SupervisorCommand.php index 857c9192..7653cbf8 100644 --- a/src/Console/SupervisorCommand.php +++ b/src/Console/SupervisorCommand.php @@ -36,7 +36,8 @@ class SupervisorCommand extends Command {--balance-max-shift=1 : The maximum number of processes to increase or decrease per one scaling} {--workers-name=default : The name that should be assigned to the workers} {--parent-id=0 : The parent process ID} - {--rest=0 : Number of seconds to rest between jobs}'; + {--rest=0 : Number of seconds to rest between jobs} + {--auto-scaling-strategy=runtime : If supervisor should scale by jobs or time to complete}'; /** * The console command description. @@ -111,12 +112,15 @@ protected function supervisorOptions() ? $this->option('backoff') : $this->option('delay'); + $balance = $this->option('balance'); + $autoScalingStrategy = $balance === 'auto' ? $this->option('auto-scaling-strategy') : null; + return new SupervisorOptions( $this->argument('name'), $this->argument('connection'), $this->getQueue($this->argument('connection')), $this->option('workers-name'), - $this->option('balance'), + $balance, $backoff, $this->option('max-time'), $this->option('max-jobs'), @@ -131,7 +135,8 @@ protected function supervisorOptions() $this->option('balance-cooldown'), $this->option('balance-max-shift'), $this->option('parent-id'), - $this->option('rest') + $this->option('rest'), + $autoScalingStrategy ); } diff --git a/src/QueueCommandString.php b/src/QueueCommandString.php index 0277b581..10984f66 100644 --- a/src/QueueCommandString.php +++ b/src/QueueCommandString.php @@ -27,7 +27,7 @@ public static function toWorkerOptionsString(SupervisorOptions $options) */ public static function toSupervisorOptionsString(SupervisorOptions $options) { - return sprintf('--workers-name=%s --balance=%s --max-processes=%s --min-processes=%s --nice=%s --balance-cooldown=%s --balance-max-shift=%s --parent-id=%s %s', + return sprintf('--workers-name=%s --balance=%s --max-processes=%s --min-processes=%s --nice=%s --balance-cooldown=%s --balance-max-shift=%s --parent-id=%s --auto-scaling-strategy=%s %s', $options->workersName, $options->balance, $options->maxProcesses, @@ -36,6 +36,7 @@ public static function toSupervisorOptionsString(SupervisorOptions $options) $options->balanceCooldown, $options->balanceMaxShift, $options->parentId, + $options->autoScalingStrategy, static::toOptionsString($options) ); } diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index d35f6ab8..0061bd35 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -158,6 +158,13 @@ class SupervisorOptions */ public $autoScale; + /** + * Indicates auto-scaling strategy should use runtime (time-to-complete) or size (total count of jobs). + * + * @var string|null + */ + public $autoScalingStrategy = null; + /** * Create a new worker options instance. * @@ -181,6 +188,7 @@ class SupervisorOptions * @param int $balanceMaxShift * @param int $parentId * @param int $rest + * @param string|null $autoScalingStrategy */ public function __construct($name, $connection, @@ -201,7 +209,9 @@ public function __construct($name, $balanceCooldown = 3, $balanceMaxShift = 1, $parentId = 0, - $rest = 0) + $rest = 0, + $autoScalingStrategy = 'runtime' + ) { $this->name = $name; $this->connection = $connection; @@ -223,6 +233,7 @@ public function __construct($name, $this->balanceMaxShift = $balanceMaxShift; $this->parentId = $parentId; $this->rest = $rest; + $this->autoScalingStrategy = $autoScalingStrategy; } /** @@ -258,6 +269,16 @@ public function autoScaling() return $this->balance === 'auto'; } + /** + * Determine if auto-scaling should be based count of jobs per queue. + * + * @return bool + */ + public function autoScaleByNumberOfJobs() + { + return $this->autoScalingStrategy === 'size'; + } + /** * Get the command-line representation of the options for a supervisor. * @@ -316,6 +337,7 @@ public function toArray() 'balanceMaxShift' => $this->balanceMaxShift, 'parentId' => $this->parentId, 'rest' => $this->rest, + 'autoScalingStrategy' => $this->autoScalingStrategy, ]; } diff --git a/tests/Feature/AddSupervisorTest.php b/tests/Feature/AddSupervisorTest.php index f2af33d3..2c064c36 100644 --- a/tests/Feature/AddSupervisorTest.php +++ b/tests/Feature/AddSupervisorTest.php @@ -28,7 +28,7 @@ public function test_add_supervisor_command_creates_new_supervisor_on_master_pro $this->assertCount(1, $master->supervisors); $this->assertSame( - 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', + 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --auto-scaling-strategy=time --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', $master->supervisors->first()->process->getCommandLine() ); } diff --git a/tests/Feature/AutoScalerTest.php b/tests/Feature/AutoScalerTest.php index e8e89dde..49c4042b 100644 --- a/tests/Feature/AutoScalerTest.php +++ b/tests/Feature/AutoScalerTest.php @@ -240,4 +240,22 @@ public function test_scaler_does_not_permit_going_to_zero_processes_despite_exce $this->assertSame(14, $supervisor->processPools['first']->totalProcessCount()); $this->assertSame(1, $supervisor->processPools['second']->totalProcessCount()); } + + public function test_scaler_assigns_more_processes_to_queue_with_more_jobs_when_using_size_strategy() + { + [$scaler, $supervisor] = $this->with_scaling_scenario(100, [ + 'first' => ['current' => 50, 'size' => 1000, 'runtime' => 10], + 'second' => ['current' => 50, 'size' => 500, 'runtime' => 1000], + ], ['autoScalingStrategy' => 'size']); + + $scaler->scale($supervisor); + + $this->assertSame(51, $supervisor->processPools['first']->totalProcessCount()); + $this->assertSame(49, $supervisor->processPools['second']->totalProcessCount()); + + $scaler->scale($supervisor); + + $this->assertSame(52, $supervisor->processPools['first']->totalProcessCount()); + $this->assertSame(48, $supervisor->processPools['second']->totalProcessCount()); + } } From a52e6216ad265ba8f8c59d692d7dfc81825dff9e Mon Sep 17 00:00:00 2001 From: Luke Kuzmish Date: Sat, 4 Mar 2023 07:22:58 -0500 Subject: [PATCH 2/7] changes autoScalingStrategy property to runtime --- tests/Feature/AddSupervisorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/AddSupervisorTest.php b/tests/Feature/AddSupervisorTest.php index 2c064c36..06d13553 100644 --- a/tests/Feature/AddSupervisorTest.php +++ b/tests/Feature/AddSupervisorTest.php @@ -28,7 +28,7 @@ public function test_add_supervisor_command_creates_new_supervisor_on_master_pro $this->assertCount(1, $master->supervisors); $this->assertSame( - 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --auto-scaling-strategy=time --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', + 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --auto-scaling-strategy=runtime --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', $master->supervisors->first()->process->getCommandLine() ); } From 9ff2b48699491b1dffdfb9259228c5459b5a5789 Mon Sep 17 00:00:00 2001 From: Luke Kuzmish Date: Sat, 4 Mar 2023 07:24:52 -0500 Subject: [PATCH 3/7] style fix --- src/SupervisorOptions.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index 0061bd35..a1612898 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -211,8 +211,7 @@ public function __construct($name, $parentId = 0, $rest = 0, $autoScalingStrategy = 'runtime' - ) - { + ) { $this->name = $name; $this->connection = $connection; $this->queue = $queue ?: config('queue.connections.'.$connection.'.queue'); From 59138bc12627f80f38d98244d3e0013210d64ab2 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 6 Mar 2023 13:03:00 -0600 Subject: [PATCH 4/7] formatting --- src/AutoScaler.php | 3 +-- src/Console/SupervisorCommand.php | 5 +++-- src/SupervisorOptions.php | 9 +-------- tests/Feature/AddSupervisorTest.php | 2 +- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/AutoScaler.php b/src/AutoScaler.php index 825d6321..26a2dfa8 100644 --- a/src/AutoScaler.php +++ b/src/AutoScaler.php @@ -104,9 +104,8 @@ protected function numberOfWorkersPerQueue(Supervisor $supervisor, Collection $q $numberOfProcesses = $supervisor->options->autoScaleByNumberOfJobs() ? ($timeToClear['size'] / $totalJobs) : ($timeToClear['time'] / $timeToClearAll); - $numberOfProcesses *= $supervisor->options->maxProcesses; - return [$queue => $numberOfProcesses]; + return [$queue => $numberOfProcesses *= $supervisor->options->maxProcesses]; } elseif ($timeToClearAll == 0 && $supervisor->options->autoScaling()) { return [ diff --git a/src/Console/SupervisorCommand.php b/src/Console/SupervisorCommand.php index 7653cbf8..aeb4773e 100644 --- a/src/Console/SupervisorCommand.php +++ b/src/Console/SupervisorCommand.php @@ -32,12 +32,12 @@ class SupervisorCommand extends Command {--sleep=3 : Number of seconds to sleep when no job is available} {--timeout=60 : The number of seconds a child process can run} {--tries=0 : Number of times to attempt a job before logging it failed} + {--auto-scaling-strategy=time : If supervisor should scale by jobs or time to complete} {--balance-cooldown=3 : The number of seconds to wait in between auto-scaling attempts} {--balance-max-shift=1 : The maximum number of processes to increase or decrease per one scaling} {--workers-name=default : The name that should be assigned to the workers} {--parent-id=0 : The parent process ID} - {--rest=0 : Number of seconds to rest between jobs} - {--auto-scaling-strategy=runtime : If supervisor should scale by jobs or time to complete}'; + {--rest=0 : Number of seconds to rest between jobs}'; /** * The console command description. @@ -113,6 +113,7 @@ protected function supervisorOptions() : $this->option('delay'); $balance = $this->option('balance'); + $autoScalingStrategy = $balance === 'auto' ? $this->option('auto-scaling-strategy') : null; return new SupervisorOptions( diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index a1612898..adf27306 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -152,14 +152,7 @@ class SupervisorOptions public $rest; /** - * Indicates if the supervisor should auto-scale. - * - * @var bool - */ - public $autoScale; - - /** - * Indicates auto-scaling strategy should use runtime (time-to-complete) or size (total count of jobs). + * Indicates whether auto-scaling strategy should use "time" (time-to-complete) or "size" (total count of jobs) strategies. * * @var string|null */ diff --git a/tests/Feature/AddSupervisorTest.php b/tests/Feature/AddSupervisorTest.php index 06d13553..2c064c36 100644 --- a/tests/Feature/AddSupervisorTest.php +++ b/tests/Feature/AddSupervisorTest.php @@ -28,7 +28,7 @@ public function test_add_supervisor_command_creates_new_supervisor_on_master_pro $this->assertCount(1, $master->supervisors); $this->assertSame( - 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --auto-scaling-strategy=runtime --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', + 'exec '.$phpBinary.' artisan horizon:supervisor my-supervisor redis --workers-name=default --balance=off --max-processes=1 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=0 --auto-scaling-strategy=time --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue="default" --sleep=3 --timeout=60 --tries=0 --rest=0', $master->supervisors->first()->process->getCommandLine() ); } From b941c40e2a1504717e06060267c1217ab92bcdc0 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 6 Mar 2023 13:03:44 -0600 Subject: [PATCH 5/7] move property --- src/SupervisorOptions.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index adf27306..0ff88085 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -39,6 +39,13 @@ class SupervisorOptions */ public $balance = 'off'; + /** + * Indicates whether auto-scaling strategy should use "time" (time-to-complete) or "size" (total count of jobs) strategies. + * + * @var string|null + */ + public $autoScalingStrategy = null; + /** * The maximum number of total processes to start when auto-scaling. * @@ -151,13 +158,6 @@ class SupervisorOptions */ public $rest; - /** - * Indicates whether auto-scaling strategy should use "time" (time-to-complete) or "size" (total count of jobs) strategies. - * - * @var string|null - */ - public $autoScalingStrategy = null; - /** * Create a new worker options instance. * From 319ec2c5f532d1ce80ccae36f168df155e8d6431 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 6 Mar 2023 13:06:59 -0600 Subject: [PATCH 6/7] fix variable --- src/SupervisorOptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index 0ff88085..b9182325 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -203,7 +203,7 @@ public function __construct($name, $balanceMaxShift = 1, $parentId = 0, $rest = 0, - $autoScalingStrategy = 'runtime' + $autoScalingStrategy = 'time' ) { $this->name = $name; $this->connection = $connection; From 597ce2395da0e28eca1eb8868647f9cc5e64cec3 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 6 Mar 2023 13:09:05 -0600 Subject: [PATCH 7/7] Update SupervisorOptions.php --- src/SupervisorOptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SupervisorOptions.php b/src/SupervisorOptions.php index b9182325..698cf14e 100644 --- a/src/SupervisorOptions.php +++ b/src/SupervisorOptions.php @@ -262,7 +262,7 @@ public function autoScaling() } /** - * Determine if auto-scaling should be based count of jobs per queue. + * Determine if auto-scaling should be based on the number of jobs on the queue instead of time-to-clear. * * @return bool */