php-runtime / runtime

A home for runtimes.
MIT License
402 stars 32 forks source link

feat(frankenphp-symfony): add kernel reboot strategy #166

Open phramz opened 10 months ago

phramz commented 10 months ago

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.

Rainrider commented 8 months ago

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

alexander-schranz commented 8 months ago

@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.

phramz commented 8 months ago

@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?

alexander-schranz commented 8 months ago

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.

phramz commented 8 months ago

@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.

Rainrider commented 8 months ago

@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.

phramz commented 8 months ago

@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.

alexander-schranz commented 8 months ago

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.