-
-
Notifications
You must be signed in to change notification settings - Fork 117
feat: fpm sapi listen always on sockets #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea.
Though it probably needs more hardening in some edge cases.
For example, if FPM is still running in the background because of some bugs, the current code is able to run by just picking a new port.
if p.Version.IsFPMServer() { | ||
if fcgi, err = fcgiclient.Dial("unix", p.addr); err == nil { | ||
break | ||
} | ||
} else { | ||
if fcgi, err = fcgiclient.Dial("tcp", p.addr); err == nil { | ||
break | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should not refactor this to have a simpler dialer internal function here.
wdyt @fabpot ?
@tucksaun do you have an idea how to make the socket bulletproof that it gets stopped 🤔. I didn't encountered a problem yet it stopped smoothly 🤔 |
if it always stops smoothly it means we did our job well with @fabpot :D More seriously, if you want to simulate this situation: Currently, this means there's a 'ghost' process, but at least restarting the web server will spawn a new FPM process and it will work for the end user.
|
it depends on the parent behavior and how process reaping is done and if children are killed or not. |
So my idea would be:
What do you think? I guess it should solve it |
I have the feeling that would complexify the server management code as it would introduce a special case for FPM. The philosophy so far has been to clean up a maximum so that the CLI does not leak things but if on a rare occasion it does this is not a big deal as it is not written for production workloads. wdyt? |
Fixes #213
Cloud based dev environments show all local listing ports and wants to forward them. But for PHP-FPM it's useless 😅
PHP-FPM is only available on Unix, so I didn't build a flag or so.
Before
After