From f0c0389beb7d371f5ae188fecae0713c391e4232 Mon Sep 17 00:00:00 2001 From: Florent Baldino Date: Mon, 13 Feb 2023 17:21:39 +0100 Subject: [PATCH] Allow to use multiple reboot strategies (#96) --- README.md | 17 +++++ phpstan.neon.dist | 2 +- .../BaldinofRoadRunnerExtension.php | 54 ++++++++++------ src/DependencyInjection/Configuration.php | 6 +- src/Reboot/ChainRebootStrategy.php | 39 ++++++++++++ src/Reboot/MaxJobsRebootStrategy.php | 5 +- tests/BaldinofRoadRunnerBundleTest.php | 49 +++++++++++++++ tests/Reboot/ChainRebootStrategyTest.php | 63 +++++++++++++++++++ tests/Worker/WorkerTest.php | 2 + 9 files changed, 214 insertions(+), 23 deletions(-) create mode 100644 src/Reboot/ChainRebootStrategy.php create mode 100644 tests/Reboot/ChainRebootStrategyTest.php diff --git a/README.md b/README.md index 3eb5c0d..dccb3d0 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ baldinof_road_runner: ``` If you are building long-running application and need to reboot it every XXX request to prevent memory leaks you can use `max_jobs` reboot strategy: + ```yaml # config/packages/baldinof_road_runner.yaml baldinof_road_runner: @@ -94,6 +95,22 @@ baldinof_road_runner: max_jobs_dispersion: 0.2 # dispersion 20% used to prevent simultaneous reboot of all active workers (kernel will rebooted between 800 and 1000 requests) ``` +You can combine reboot strategies: + + +```yaml +# config/packages/baldinof_road_runner.yaml +baldinof_road_runner: + kernel_reboot: + strategy: [on_exception, max_jobs] + allowed_exceptions: + - Symfony\Component\HttpKernel\Exception\HttpExceptionInterface + - Symfony\Component\Serializer\Exception\ExceptionInterface + - App\Exception\YourDomainException + max_jobs: 1000 + max_jobs_dispersion: 0.2 +``` + ## Events diff --git a/phpstan.neon.dist b/phpstan.neon.dist index c09111c..5204a71 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -12,7 +12,7 @@ parameters: ignoreErrors: - message: '#Access to protected property Symfony\\Component\\HttpKernel\\Kernel::\$startTime.#' path: 'src/Http/KernelHandler.php' - - message: '#Cannot call method arrayNode#' + - message: '#Call to an undefined method#' path: 'src/DependencyInjection' - message: '#Cannot cast mixed to string.#' path: 'src/Integration/PHP/NativeSessionMiddleware.php' diff --git a/src/DependencyInjection/BaldinofRoadRunnerExtension.php b/src/DependencyInjection/BaldinofRoadRunnerExtension.php index 57d5158..b8c4c4e 100644 --- a/src/DependencyInjection/BaldinofRoadRunnerExtension.php +++ b/src/DependencyInjection/BaldinofRoadRunnerExtension.php @@ -14,6 +14,7 @@ use Baldinof\RoadRunnerBundle\Integration\Sentry\SentryMiddleware; use Baldinof\RoadRunnerBundle\Integration\Symfony\ConfigureVarDumperListener; use Baldinof\RoadRunnerBundle\Reboot\AlwaysRebootStrategy; +use Baldinof\RoadRunnerBundle\Reboot\ChainRebootStrategy; use Baldinof\RoadRunnerBundle\Reboot\KernelRebootStrategyInterface; use Baldinof\RoadRunnerBundle\Reboot\MaxJobsRebootStrategy; use Baldinof\RoadRunnerBundle\Reboot\OnExceptionRebootStrategy; @@ -52,25 +53,42 @@ public function load(array $configs, ContainerBuilder $container): void $this->loadDebug($container); } - if ($config['kernel_reboot']['strategy'] === Configuration::KERNEL_REBOOT_STRATEGY_ALWAYS) { - $container - ->register(KernelRebootStrategyInterface::class, AlwaysRebootStrategy::class) - ->setAutoconfigured(true); - } elseif ($config['kernel_reboot']['strategy'] === Configuration::KERNEL_REBOOT_STRATEGY_ON_EXCEPTION) { - $container - ->register(KernelRebootStrategyInterface::class, OnExceptionRebootStrategy::class) - ->addArgument($config['kernel_reboot']['allowed_exceptions']) - ->addArgument(new Reference(LoggerInterface::class)) - ->setAutoconfigured(true) - ->addTag('monolog.logger', ['channel' => self::MONOLOG_CHANNEL]); - } elseif ($config['kernel_reboot']['strategy'] === Configuration::KERNEL_REBOOT_STRATEGY_MAX_JOBS) { - $container - ->register(KernelRebootStrategyInterface::class, MaxJobsRebootStrategy::class) - ->addArgument($config['kernel_reboot']['max_jobs']) - ->addArgument($config['kernel_reboot']['max_jobs_dispersion']) - ->setAutoconfigured(true); + $strategies = $config['kernel_reboot']['strategy']; + $strategyServices = []; + + foreach ($strategies as $strategy) { + if ($strategy === Configuration::KERNEL_REBOOT_STRATEGY_ALWAYS) { + $strategyService = (new Definition(AlwaysRebootStrategy::class)) + ->setAutoconfigured(true); + } elseif ($strategy === Configuration::KERNEL_REBOOT_STRATEGY_ON_EXCEPTION) { + $strategyService = (new Definition(OnExceptionRebootStrategy::class)) + ->addArgument($config['kernel_reboot']['allowed_exceptions']) + ->addArgument(new Reference(LoggerInterface::class)) + ->setAutoconfigured(true) + ->addTag('monolog.logger', ['channel' => self::MONOLOG_CHANNEL]); + } elseif ($strategy === Configuration::KERNEL_REBOOT_STRATEGY_MAX_JOBS) { + $strategyService = (new Definition(MaxJobsRebootStrategy::class)) + ->addArgument($config['kernel_reboot']['max_jobs']) + ->addArgument($config['kernel_reboot']['max_jobs_dispersion']) + ->setAutoconfigured(true); + } else { + $strategyService = new Reference($strategy); + } + + $strategyServices[] = $strategyService; + } + + if (\count($strategyServices) > 1) { + $container->register(KernelRebootStrategyInterface::class, ChainRebootStrategy::class) + ->setArguments([$strategyServices]); } else { - $container->setAlias(KernelRebootStrategyInterface::class, $config['kernel_reboot']['strategy']); + $strategy = $strategyServices[0]; + + if ($strategy instanceof Reference) { + $container->setAlias(KernelRebootStrategyInterface::class, (string) $strategy); + } else { + $container->setDefinition(KernelRebootStrategyInterface::class, $strategy); + } } $container->setParameter('baldinof_road_runner.middlewares', $config['middlewares']); diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 8fbaf2f..6eee594 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -28,7 +28,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->arrayNode('kernel_reboot') ->addDefaultsIfNotSet() ->children() - ->scalarNode('strategy') + ->arrayNode('strategy') ->info(sprintf( 'Possible values are "%s", "%s", "%s" or any service that implements "%s"/', self::KERNEL_REBOOT_STRATEGY_ALWAYS, @@ -36,7 +36,9 @@ public function getConfigTreeBuilder(): TreeBuilder self::KERNEL_REBOOT_STRATEGY_MAX_JOBS, KernelRebootStrategyInterface::class )) - ->defaultValue(self::KERNEL_REBOOT_STRATEGY_ON_EXCEPTION) + ->defaultValue([self::KERNEL_REBOOT_STRATEGY_ON_EXCEPTION]) + ->beforeNormalization()->castToArray()->end() + ->scalarPrototype()->end() ->end() ->arrayNode('allowed_exceptions') ->info('Only used when `reboot_kernel.strategy: on_exception`. Exceptions defined here will not cause kernel reboots.') diff --git a/src/Reboot/ChainRebootStrategy.php b/src/Reboot/ChainRebootStrategy.php new file mode 100644 index 0000000..d369735 --- /dev/null +++ b/src/Reboot/ChainRebootStrategy.php @@ -0,0 +1,39 @@ + + */ + private iterable $strategies; + + /** + * @param iterable $strategies + */ + public function __construct(iterable $strategies) + { + $this->strategies = $strategies; + } + + public function shouldReboot(): bool + { + foreach ($this->strategies as $strategy) { + if ($strategy->shouldReboot()) { + return true; + } + } + + return false; + } + + public function clear(): void + { + foreach ($this->strategies as $strategy) { + $strategy->clear(); + } + } +} diff --git a/src/Reboot/MaxJobsRebootStrategy.php b/src/Reboot/MaxJobsRebootStrategy.php index d82c4ac..797a64a 100644 --- a/src/Reboot/MaxJobsRebootStrategy.php +++ b/src/Reboot/MaxJobsRebootStrategy.php @@ -12,13 +12,14 @@ class MaxJobsRebootStrategy implements KernelRebootStrategyInterface public function __construct(int $maxJobs, float $dispersion) { $minJobs = $maxJobs - (int) round($maxJobs * $dispersion); - $this->maxJobs = \random_int($minJobs, $maxJobs); + $this->maxJobs = random_int($minJobs, $maxJobs); } public function shouldReboot(): bool { if ($this->jobsCount < $this->maxJobs) { - $this->jobsCount++; + ++$this->jobsCount; + return false; } diff --git a/tests/BaldinofRoadRunnerBundleTest.php b/tests/BaldinofRoadRunnerBundleTest.php index fd1abe6..85f6447 100644 --- a/tests/BaldinofRoadRunnerBundleTest.php +++ b/tests/BaldinofRoadRunnerBundleTest.php @@ -11,6 +11,11 @@ use Baldinof\RoadRunnerBundle\Integration\PHP\NativeSessionMiddleware; use Baldinof\RoadRunnerBundle\Integration\Sentry\SentryMiddleware; use Baldinof\RoadRunnerBundle\Integration\Symfony\StreamedResponseListener; +use Baldinof\RoadRunnerBundle\Reboot\AlwaysRebootStrategy; +use Baldinof\RoadRunnerBundle\Reboot\ChainRebootStrategy; +use Baldinof\RoadRunnerBundle\Reboot\KernelRebootStrategyInterface; +use Baldinof\RoadRunnerBundle\Reboot\MaxJobsRebootStrategy; +use Baldinof\RoadRunnerBundle\Reboot\OnExceptionRebootStrategy; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\SentryBundle; @@ -184,6 +189,50 @@ public function test_it_removes_session_middleware() $this->assertFalse($c->has(NativeSessionMiddleware::class)); } + public function test_it_supports_single_strategy() + { + $k = $this->getKernel([ + 'baldinof_road_runner' => [ + 'kernel_reboot' => [ + 'strategy' => 'always', + ], + ], + ]); + + $k->boot(); + + $c = $k->getContainer()->get('test.service_container'); + + $this->assertInstanceOf(AlwaysRebootStrategy::class, $c->get(KernelRebootStrategyInterface::class)); + } + + public function test_it_supports_multiple_strategies() + { + $k = $this->getKernel([ + 'baldinof_road_runner' => [ + 'kernel_reboot' => [ + 'strategy' => ['on_exception', 'max_jobs'], + ], + ], + ]); + + $k->boot(); + + $c = $k->getContainer()->get('test.service_container'); + + $strategy = $c->get(KernelRebootStrategyInterface::class); + + $this->assertInstanceOf(ChainRebootStrategy::class, $strategy); + + $strategies = (function () { + return $this->strategies; + })->bindTo($strategy, ChainRebootStrategy::class)(); + + $this->assertCount(2, $strategies); + $this->assertInstanceOf(OnExceptionRebootStrategy::class, $strategies[0]); + $this->assertInstanceOf(MaxJobsRebootStrategy::class, $strategies[1]); + } + /** * @param BundleInterface[] $extraBundles */ diff --git a/tests/Reboot/ChainRebootStrategyTest.php b/tests/Reboot/ChainRebootStrategyTest.php new file mode 100644 index 0000000..291dd11 --- /dev/null +++ b/tests/Reboot/ChainRebootStrategyTest.php @@ -0,0 +1,63 @@ +assertFalse($strategy->shouldReboot()); + } + + public function test_it_reboot_if_one_strategy_reboot() + { + $strategy = new ChainRebootStrategy([ + $this->createStrategy(true), + $this->createStrategy(false), + ]); + + $this->assertTrue($strategy->shouldReboot()); + + $strategy = new ChainRebootStrategy([ + $this->createStrategy(false), + $this->createStrategy(true), + ]); + } + + public function test_it_does_not_reboot_if_no_strategy_reboot() + { + $strategy = new ChainRebootStrategy([ + $this->createStrategy(false), + $this->createStrategy(false), + ]); + $this->assertFalse($strategy->shouldReboot()); + } + + private function createStrategy(bool $shouldReboot): KernelRebootStrategyInterface + { + return new class($shouldReboot) implements KernelRebootStrategyInterface { + private bool $shouldReboot; + + public function __construct(bool $shouldReboot) + { + $this->shouldReboot = $shouldReboot; + } + + public function shouldReboot(): bool + { + return $this->shouldReboot; + } + + public function clear(): void + { + } + }; + } +} diff --git a/tests/Worker/WorkerTest.php b/tests/Worker/WorkerTest.php index 5331bb6..d8ee6a5 100644 --- a/tests/Worker/WorkerTest.php +++ b/tests/Worker/WorkerTest.php @@ -4,6 +4,7 @@ namespace Tests\Baldinof\RoadRunnerBundle\Worker; +use AllowDynamicProperties; use Baldinof\RoadRunnerBundle\Event\WorkerExceptionEvent; use Baldinof\RoadRunnerBundle\Event\WorkerKernelRebootedEvent; use Baldinof\RoadRunnerBundle\Event\WorkerStopEvent; @@ -27,6 +28,7 @@ use Symfony\Component\HttpKernel\RebootableInterface; use Symfony\Component\HttpKernel\TerminableInterface; +#[\AllowDynamicProperties] class WorkerTest extends TestCase { use ProphecyTrait;