Skip to content
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

[Draft] Add mpm_event / FPM support to apache variant #785

Closed
wants to merge 6 commits into from

Conversation

mildsunrise
Copy link

@mildsunrise mildsunrise commented Jan 31, 2019

On apache images, this adds an apache-fpm command that starts Apache + FPM simultaneously (taking care of switching to mpm_event + proxy_fcgi). As said in #742, this is the upstream recommended setup.

To switch to this setup, all users have to do is CMD ["apache-fpm"] in their Dockerfile.

Implementation

The first commit just copies build flags and configuration code from fpm variant.
The second commit adds apache-fpm. Things to note:

  • FPM listens on a UNIX socket, not TCP as in original variant. This improves performance.
  • access_log is disabled by default on FPM to reduce log spam (only Apache access log is shown).
  • runit is used to run both processes simultaneously. It's far lightweight and simple to setup than i.e. supervisord, and only consumes ~1MB of RAM in my tests. It's used in phusion-baseimage as well.
  • apache-fpm disables mpm_prefork + php and enables mpm_event + proxy_fcgi before starting everything. This can be disabled with environment variable APACHE_KEEP_MODS if the user needs absolute freedom over loaded modules.

@mildsunrise mildsunrise force-pushed the apache-fpm branch 2 times, most recently from 8ac432d to 79b5d63 Compare February 1, 2019 01:19
@mildsunrise
Copy link
Author

Rebased! Also, just FYI: it turns out Apache (when in prefork) kills its whole process group when it is terminated. So, if run from a script, killing Apache will make it kill the script.

@mildsunrise
Copy link
Author

mildsunrise commented Feb 1, 2019

Missing things:

  • Make this work for PHP5, where the a2dismod php7 won't work. PHP5 support no longer needed, can we leave php7 hardcoded?
  • Make this work with any user.
  • Do we want to allow running FPM and Apache as different users? Why?

@@ -3,7 +3,7 @@ ENV APACHE_ENVVARS $APACHE_CONFDIR/envvars

RUN set -eux; \
apt-get update; \
apt-get install -y --no-install-recommends apache2; \
apt-get install -y --no-install-recommends apache2 runit; \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why is runit here? Can't Apache launch and manage the FPM process directly?

(As much as I love the idea of FPM+Apache automatically, an extra process supervisor like runit is going to be DOA for official images.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea. AFAIK, Apache doesn't have the functionality to launch / supervise FPM.
I'll remove the supervisor entirely then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, we're caught between mod_fastcgi and mod_fcgid -- we only have mod_proxy_fcgi by default, which does not support actually supervising a process (where both the former external module and the latter module explicitly do).

See https://stackoverflow.com/a/38225224/433558 and/or https://httpd.apache.org/mod_fcgid/ (especially "which starts a sufficient number instances of the CGI program to handle concurrent requests").

It would appear that installing libapache2-mod-fcgid adds a very tiny amount of disk space overhead, and the benefits of having Apache directly launch, supervise, restart, and maintain etc the FPM process(es) are going to be the only way this can really work -- it won't be acceptable for the official images to have something like runit or a script doing that supervision of multiple processes, especially for what amounts to an optional feature of the image.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think PHP-FPM is capable of running supervised by another process. PHP-FPM is already a process manager and handle thinks like shared memory between processes for cache. Making it run supervised by another process looks impossible to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that were the case, it wouldn't be possible to use via runit, systemd, Docker itself, or even a shell.

However, I can't find anyone crazy enough to try and use it via FcgidWrapper, so I'm afraid this might be a dream we can't actually accomplish ATM (which isn't good news for this PR). 😞

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @jvasseur. So the main options here are (1) have Apache's mod_fcgid/mod_fastcgi launch FCGI processes, or (2) use PHP-FPM to launch FCGI processes.

As stated in the official docs, PHP-FPM is an «alternative PHP FastCGI implementation with some additional features (mostly) useful for heavy-loaded sites». According to some people, PHP-FPM «provides every benefit that fcgid has, with the added advantage of a shared opcode cache for all processes». This would explain why PHP-FPM is generally believed to be more memory-efficient. PHP-FPM is used by default in Fedora.

I tried to find if PHP has a recommendation of PHP-FPM vs standard FCGI, but nothing came up. There is Apache documentation for both approaches, but they are never directly compared. However fcgid would make things easier in the Dockerfile.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it might be technically possible to use it with mod_fcgid if we restrict mod_fcgid to run one and only one process (anything else and we would loose all the benefits of php-fpm), but that fells quite hacky.

But I don't think having a supervising process in the image is a good way to go to, if you need to processes you can use two container.

Copy link
Author

@mildsunrise mildsunrise Feb 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it might be technically possible to use it with mod_fcgid if we restrict mod_fcgid to run one and only one process (anything else and we would loose all the benefits of php-fpm), but that fells quite hacky.

True! I'll do some tests with that, but yeah it feels quite hacky.

Simply running php-fpm alongside Apache without any supervision seems to work well. PHP-FPM is itself a supervisor, so there's a very low risk of it dying (and in that case, it's up to Docker to restart the container).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you need to processes you can use two container

I usually agree with this, but CGI/FCGI are inter-process interfaces. The FCGI process expects to operate on the same filesystem as the webserver. For some apps it's feasible to run PHP-FPM on a different machine / container (or a farm!) and that's why php:fpm is for. But in other cases, it can be a challenge.

We've tried to migrate some of our images to the dual-container system (php:fpm + apache) and it can be a real nightmare. I think php:apache should provide a transparent option for a more efficient configuration, be it mod_fcgid, ZTS or PHP-FPM (even if the latter can be split into 2 containers).

@mildsunrise
Copy link
Author

I've removed the extra supervision! Now we just start both processes, and if one of them crashes the container fails. (Also sorry about force-pushing, I had to rebase again.)

@mivanovaxway
Copy link

Hope this gets merged. I've been thinking about how slow and obsolete mod_php with mpm_prefork is since I first tried the PHP/Apache images. ZTS seems like another possible alternative but I don't think it's tested enough. Having PHP-FPM/Apache in the same image and then running in the container makes perfect sense since we don't have the option to start an HTTP server in PHP code(nothing that's official and production ready anyway) and get rid of Apache.

@amq
Copy link

amq commented Dec 18, 2019

This looks great, would switch immediately. I hope it would be also used for the wordpress image, at least as an optional tag.

@mildsunrise
Copy link
Author

This will probably never get merged because it launches 2 processes, not one.
I still hold that Docker should make an exception here, because FCGI is an inter-process protocol.

The four ways of serving PHP with Apache are:

  • prefork + modphp
  • event + zts
  • event + fcgi
  • event + fpm

It's sad that on 2020, the only official option is the inefficient prefork. If the last option (this PR) can't be merged because of the one-process rule, can we at least implement the third or second one?

@amq
Copy link

amq commented Dec 18, 2019

Strictly speaking, current apache images also launch multiple processes. I think event + fpm is definitely worth an exception.

@yosifkit
Copy link
Member

Strictly speaking, current apache images also launch multiple processes

The Dockerfile is concerned with a single process to determine container lifecycle and what apache manages under the covers to launch itself and php does not matter from the perspective of dockerd and containerd.

We don't want to implement or use a supervisor to monitor multiple processes in the container. I did quite a bit of research to create a container running haproxy with a syslog server and nothing was adequate for use in a container; we'd need similar to the requirements here (swapping haproxy with apache and syslog with php-fpm).

I had a PR for zts + event in #753, but php recommends against using it in production environments (https://www.php.net/manual/en/faq.installation.php#faq.installation.apache2).

@mildsunrise
Copy link
Author

We don't want to implement or use a supervisor to monitor multiple processes in the container.

I don't see why you need a supervisor, it's not used in the last version of this PR.

we'd need similar to the requirements here

This is a different situation; there's not a main process (haproxy) and a side process (rsyslog), both processes are equally important. So we can't tie one process' lifecycle to the other.

Instead, the container fails when either Apache or FPM fails. Other than that, the requirements match for a suitable init:

  • Starts both processes.
  • Forwards stop signals to both processes.
  • Waits for both processes to stop.
  • Exits when either of them fails.
  • Collects zombies.

&& cd /usr/local/etc \
&& if [ -d php-fpm.d ]; then \
# for some reason, upstream's php-fpm.conf.default has "include=NONE/etc/php-fpm.d/*.conf"
sed 's!=NONE/!=!g' php-fpm.conf.default | tee php-fpm.conf > /dev/null; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| tee php-fpm.conf > /dev/null seems super pointless, just redirect to > php-fpm.conf directly?

Copy link
Author

@mildsunrise mildsunrise Dec 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! These lines were copied directly from the current FPM dockerfile.
I think these corrections should be done there first, in order to not maintain different versions of the snippet.

apache-Dockerfile-block-2 Show resolved Hide resolved
@tianon
Copy link
Member

tianon commented Dec 23, 2019

I don't see why you need a supervisor, it's not used in the last version of this PR.

It is though -- https://github.com/docker-library/php/pull/785/files#diff-04a3b22acd8287c52a1190ede5817188R21-R30 is the supervisor code (it's a bash-based supervisor instead of one of the dedicated supervisor projects). 😕

This is a different situation; there's not a main process (haproxy) and a side process (rsyslog), both processes are equally important. So we can't tie one process' lifecycle to the other.

The fact that we can't tie one process lifecycle to the other is exactly why this needs a supervisor (to ensure that they either both stay running, or neither stays running). 😞

@mildsunrise
Copy link
Author

It is though -- https://github.com/docker-library/php/pull/785/files#diff-04a3b22acd8287c52a1190ede5817188R21-R30 is the supervisor code (it's a bash-based supervisor instead of one of the dedicated supervisor projects). 😕

Oh, it seems we had different definitions of what a supervisor is 🤔 Okay, noted

The fact that we can't tie one process lifecycle to the other is exactly why this needs a supervisor (to ensure that they either both stay running, or neither stays running). 😞

And that's what the bash supervisor does, I think.
But then I don't understand the reasoning behind «nothing was adequate for use in a container». What technical / practical issues arise with a supervisor like this?

@tianon
Copy link
Member

tianon commented Dec 27, 2019

Sure, as one example of where this starts to get complicated, what do we do with a signal that means something to one process or the other such as SIGHUP or SIGUSR1?

@mildsunrise
Copy link
Author

To send that signal you can docker exec <...> killall -USR1 php-fpm, right? I know it's more verbose than docker kill -s USR1 <...>, but I don't see a big deal 🤔

To be 100% correct you can use a pidfile, i.e. docker exec <...> xargs -a/php-fpm.pid kill -USR1

@mildsunrise
Copy link
Author

mildsunrise commented Dec 30, 2019

I mean, the supervisor should ignore these signals because they're process-specific.

@yosifkit
Copy link
Member

To send that signal you can docker exec <...> killall -USR1 php-fpm, right? I know it's more verbose than docker kill -s USR1 <...>, but I don't see a big deal 🤔

I mean, the supervisor should ignore these signals because they're process-specific.

Unfortunately that breaks down the portability of the container since I don't expect orchestrators like Kubernetes to do special hacks to send a specific signal to a specific process of a container when they don't even have an endpoint for signals to Pods: kubernetes/kubernetes#24957.

@mildsunrise
Copy link
Author

But why would an orchestrator send a process-specific signal to a container?

@mildsunrise
Copy link
Author

Oh, I see the point... Yes, then it's understandable that you want to have a main process (that receives forwarded signals) and a secondary one.

@gustavovalverde
Copy link

So, do we have any alternative to have a single Dockerfile with PHP with Apache and FPM?

@yosifkit
Copy link
Member

yosifkit commented Sep 2, 2021

Thank you for your contribution and apologies for letting this sit so long.

Since there is no way to have both apache2 and php-fpm in a single container without a process supervisor, we have no intentions of maintaining such an image in the official-images.

Users could likely be able to run the two as separate containers, by using a php:fpm image along with either a php:apache or httpd image with mpm_event and other config applied. Alternatively, use the nginx image with php:fpm.

@mildsunrise
Copy link
Author

Using nginx may not be an alternative, as PHP projects often have dependencies on the dreaded .htaccess.
That "likely" also doesn't match our experience, since PHP projects tend to mix code with assets & uploads.

If I understand correctly, the official response to a Apache/PHP webserver is "you can either use the old & inefficient setup (prefork image), or maybe you're able to run the setup as two separate images".

I understood the reasons, but it's still a very sad outcome from the user's perspective.

@mildsunrise mildsunrise deleted the apache-fpm branch September 3, 2021 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants