dunglas / frankenphp

🧟 The modern PHP app server
https://frankenphp.dev
MIT License
6.99k stars 243 forks source link

Auto restart based on condition #671

Closed lermontex closed 7 months ago

lermontex commented 8 months ago

Describe you feature request

Is your feature request related to a problem? Please describe. There are some known issues with connecting to the database #290, #233, #438, and there may be other reasons to limit the web server's runtime

Describe the solution you'd like How about implementing the ability to automatically restart the web server when certain limits are reached? A similar feature is supported in Symfony Messenger:

Don't Let Workers Run Forever Some services (like Doctrine's EntityManager) will consume more memory over time. So, instead of allowing your worker to run forever, use a flag like messenger:consume --limit=10 to tell your worker to only handle 10 messages before exiting (then the process manager will create a new process). There are also other options like --memory-limit=128M and --time-limit=3600.

Stopping Workers That Encounter Errors If a worker dependency like your database server is down, or timeout is reached, you can try to add reconnect logic, or just quit the worker if it receives too many errors with the --failure-limit option of the messenger:consume command.

https://symfony.com/doc/current/messenger.html#deploying-to-production

This way it would be possible to set up several containers using Docker Swarm (for example) and achieve uninterrupted operation of the web server

dunglas commented 7 months ago

The DB issue has been fixed in Symfony (https://github.com/symfony/symfony/pull/53214) and it is now possible to implement an auto-restart strategy userland (return the 0 status code in the worker script if the restart is intended, a number superior to 0 if it is not intended). This has been implemented in Laravel Octane (memory and number of requests) and Symfony Runtime (number of requests).

Closing as done 🎉

Rainrider commented 7 months ago

This has not been fixed yet actually. The linked Symfony issue depends on https://github.com/doctrine/DoctrineBundle/pull/1739 which has not been merged yet. Also the "fix" in the Symfony Runtime is not sufficient - if you don't get the amount of requests needed to restart the runtime within the connection idle time threshold of your database, you will still get the reported errors. Please correct me if I'm wrong, but you have to disable worker mode for now or increase the idle time threshold settings for your database (i.e. wait_timeout and/or interactive_timeout for MariaDB/MySQL).

dunglas commented 7 months ago

Indeed, the fix in the Doctrine bundle is still pending but will be merged soon.

Anyway, it's good practice to set a shorter timeout on the client side (PDO) than on the server side (MySQL server), and this totally prevents this issue.

tamcy commented 5 months ago

This makes me wonder if there has been any consideration on support of resetting the worker after it reaches a max ttl, or max idle ttl, as this can happen on any external service, not just MySQL (or any database service)?

dunglas commented 5 months ago

This can be implemented in the worker script directly!

tamcy commented 5 months ago

My apology because I'm not following it very closely; for Symfony integration I think this means we'd implement our own Runner and Runtime base on https://github.com/php-runtime/frankenphp-symfony/tree/main, right?

dunglas commented 5 months ago

Exactly! Don't hesitate to contribute such options to this package by the way. I'm sure this can be useful for other users.

tamcy commented 5 months ago

@dunglas From what I had tried, \frankenphp_handle_request() will block and wait for a request to be handled. It is the worker that is left idle for too long (such that it exceeds the wait_timeout in MySQL) that causes the MySQL gone away (and similar timeout error). It'd be too late to break the loop after the exception occurs, so practically it doesn't seem possible to check for a "idle time" in the Runner code?

Blackvz commented 4 months ago

We currently have the same issue

dunglas commented 4 months ago

You could add your check at the beginning of the closure, right? Technically this will block more than the idle time, but this will have practically the same effect.