runtime icon indicating copy to clipboard operation
runtime copied to clipboard

feat(frankenphp-symfony): add kernel reboot strategy

Open phramz opened this issue 1 year ago • 8 comments

I was facing weird side effects regarding PHP sessions and Doctrine ORM running Symfony 6.x and 7.x applications in FrankenPHP worker-mode.

Rebooting the kernel between requests fixed it.

I had this kind of issues back than using Roadrunner as well, so the reboot strategy implementation is inspired by https://github.com/Baldinof/roadrunner-bundle.

Though I guess an on_exception strategy does not make to much sense on a runtime layer (since FrankenPHP will kill the worker if an exception is not handled), it's just 'never' and 'always' for now.

phramz avatar Jan 19 '24 11:01 phramz

Hm, does this not offset the benefits of the worker mode?

Rainrider avatar Mar 05 '24 22:03 Rainrider

@phramz that sounds like an issue that some services may forgot to implement the KernelResetInterface. In the worker mode the kernel should be keep bootet and only services resetted.

alexander-schranz avatar Mar 06 '24 07:03 alexander-schranz

@Rainrider Since the reboot takes place between requests, it will take some extra time until the worker is able the handle the next request which might impact performance in case all workers are busy (e.g. under high load). This should not impact other "benefits" of the worker mode.

The kernel reboot mode defaults to never. Where always is just a plan B so one is able to run applications shipping bundles/libs that introduce side-effects by not cleaning up their global states between requests. Since many developers assumed PHP will handle clean up and libs were not developed having long running process execution in mind.

@alexander-schranz which KernelResetInterface are you referring to?

phramz avatar Mar 06 '24 09:03 phramz

The https://github.com/symfony/symfony/blob/7.1/src/Symfony/Contracts/Service/ResetInterface.php (service tag kernel.reset) is responsible that a Symfony Service clean it self up without have to reboot the Kernel. If you have problems with session it may an issue with the sessions you may access the session differently then over the Symfony objects? Symfony itself does there the cleanup of the session in its Listeners.

alexander-schranz avatar Mar 06 '24 11:03 alexander-schranz

@alexander-schranz I guess that might be the general root cause for the issues. I had this type of side effects in two different SF/Doctrine projects so far using RoadRunner as well as FrankenPHP in worker mode recently.

For me it pretty much looks related to https://github.com/Baldinof/roadrunner-bundle/issues/116

Unfortunately I didn't have enough time to do perform an in the depth debugging of all components being involved, so I went for a kernel reboot between requests (luckily RR supported it) that solved it for me.

phramz avatar Mar 06 '24 12:03 phramz

@phramz is there any benefit left other than not having to pay start-up costs for the worker itself? Please excuse my lack of knowledge in this regard.

Depending on your answer it might be better to just disable worker mode instead.

Rainrider avatar Mar 06 '24 17:03 Rainrider

@Rainrider The biggest advantage of using worker-mode is the performance gain. This is due to request hit a warm (running) application. That eliminates expensive overhead like parsing, class loading, etcpp. The trade-off is that this introduces a global application state prone to side-effects between requests that needs to be addressed by all components by let's say cleaning up after themselves (at the latest) when the request/response cycle is finished.

phramz avatar Mar 08 '24 09:03 phramz

That eliminates expensive overhead like parsing, class loading, etcpp.

Class loading and parsing is what the opcache is in PHP for and you can optimize that part via opcache.preload. The worker mode really advantages come in place wehre the Kernel is kept booted and services instanced and only need to reset their states instead of rebuilding all services again. I think there is not much difference if you use reboot after every request and not using the worker mode. Maybe reboot requires even some more resources as it need to destroy more instead of killing all directly.

The easiest way to find where you need to add a ResetInterface is to search for something like \$this\-\>(.*) = which is not inside a constructor. Services should in best practices be stateless (readonly). That is also why in frameworks like Symfony Request object are not longer services like in old versions.

Keep in mind if you use example async messenge:consume via Symfony Messenger you need to take care of the same things via the ResetInterface, to make sure the services are between 2 consumed messages correctly reseted.

alexander-schranz avatar Mar 08 '24 11:03 alexander-schranz

I think it could be interesting to have the on_error strategy.

For example

            if ($this->kernel instanceof RebootableInterface && ('on_error' === $this->kernelReboot) && $sfResponse->getStatusCode() >= 500) {
                $this->kernel->reboot(null);
            }

In our project, we encountered EntityManagerClosed exceptions. Here's our setup:

  • We expose a public API.
  • An ApiResource on a User entity allows users to POST new users.
  • We use a UniqueEntity constraint: #[UniqueEntity(fields: ['email', 'structure'])].
  • We have an ORM UniqueConstraint: #[ORM\UniqueConstraint(columns: ['email'])].

We observed some requests ending with a 500 status code due to a UniqueConstraintViolation, indicating that a user with the given email already exists. This occurred when the same request was sent twice simultaneously. The first request completed with a 201 response, but the second resulted in a 500 response.

In worker mode, if the entity manager is closed, all requests handled by the worker result in an EntityManagerClosed exception. Rebooting the kernel after a 500 response helped us avoid these errors.

vincenttouzet avatar Feb 26 '25 22:02 vincenttouzet