Ensure Autoscaler never reduces processes to less than 1 (#1221)

* Don't tolerate negative `$maxUpShift` * Fix autoscaling letting processes hit 0 * Fix autoscaling letting processes hit 0
laravel · Nov 25, 2022 · 01b26da · 01b26da
1 parent 9e65557
commit 01b26da
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 1 deletion.
diff --git a/src/AutoScaler.php b/src/AutoScaler.php
@@ -132,7 +132,7 @@ protected function scalePool(Supervisor $supervisor, $pool, $workers)
 
         if ($desiredProcessCount > $totalProcessCount) {
             $maxUpShift = min(
-                $supervisor->options->maxProcesses - $supervisor->totalProcessCount(),
+                max(0, $supervisor->options->maxProcesses - $supervisor->totalProcessCount()),
                 $supervisor->options->balanceMaxShift
             );
 

diff --git a/tests/Feature/AutoScalerTest.php b/tests/Feature/AutoScalerTest.php
@@ -218,4 +218,26 @@ public function test_scaler_considers_max_shift_and_attempts_to_get_closer_to_pr
         $this->assertEquals(100, $supervisor->processPools['first']->totalProcessCount());
         $this->assertEquals(50, $supervisor->processPools['second']->totalProcessCount());
     }
+
+    public function test_scaler_does_not_permit_going_to_zero_processes_despite_exceeding_max_processes()
+    {
+        $external = Mockery::mock(SystemProcessCounter::class);
+        $external->shouldReceive('get')->with('name')->andReturn(5);
+        $this->app->instance(SystemProcessCounter::class, $external);
+
+        [$scaler, $supervisor] = $this->with_scaling_scenario(15, [
+            'first' => ['current' => 16, 'size' => 1, 'runtime' => 1],
+            'second' => ['current' => 1, 'size' => 1, 'runtime' => 1],
+        ], ['minProcesses' => 1]);
+
+        $scaler->scale($supervisor);
+
+        $this->assertSame(15, $supervisor->processPools['first']->totalProcessCount());
+        $this->assertSame(1, $supervisor->processPools['second']->totalProcessCount());
+
+        $scaler->scale($supervisor);
+
+        $this->assertSame(14, $supervisor->processPools['first']->totalProcessCount());
+        $this->assertSame(1, $supervisor->processPools['second']->totalProcessCount());
+    }
 }