Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:53727
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.83.170 as permitted sender)
MIME-Version: 1.0
Sender: giovanni.g@gmail.com
Date: Sun, 3 Jul 2011 02:50:15 +0200
Message-ID: <CAML1Pf7z1xYL1FGxv9Lb0pq3B=oy0DDSKE3nLQgu2zn-q-Ce-Q@mail.gmail.com>
To: internals@lists.php.net
Content-Type: multipart/mixed; boundary=00504502ccb248d31b04a71fa20a
Subject: Big patch for FPM (config and initialization)
From: giovanni@giacobbi.net (Giovanni Giacobbi)

--00504502ccb248d31b04a71fa20a
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Greetings dear devs!

A few days ago I made my first nginx+php-fpm setup, and I soon
realized some gaps of the current FPM implementation. First of all,
the lack of documentation, which of course I know I cannot complain
about, but this forced me to dig into the source code which in turn
motivated me to write this patch, which is good.

The patch (attached) is for the current PHP 5.3 svn branch, but if you
are interested in merging it in I will of course port it to 5.4
branch. I don't see particular reasons to apply it to 5.3, I was
working on it because I'm planning to use it in my production
environments. It would be really nice if it could make it in the 5.4
because it contains also some name changes in the ini file variables
(with BC of course).

The biggest and most interesting change is for sure the introduction
of a [defaults] config section. More about this below, first the
complete changelog of my patch:

Detailed changelog of the patch:
- Renamed options "pm.status_path", "ping.path", "ping.response" into
a new logical group "diagnostics", so they are now respectively:
=A0=A0=A0 "diagnostics.status_path", "diagnostics.ping_path",
"diagnostics.ping_response".
- Reordered in a more logical way the pool ini options, from a
most-likely-to-be-customized first to the least. Usually when you edit
config file you have big concentration on the first few settings, then
you go like "blah blah defaults defaults" and so on, so this kind of
order makes sense.
- Aligned all the code listings of pool options with the "official"
order to ease auditing. The "official" order is the one reported in
the struct definition in fpm_conf.h, which of course is the same as
the php-fpm.conf.in config template
- Improved error messages. A better message helps new adopters to get
started quickly, at the beginning I was really puzzled in front of
some not very helpful messages.
- Introduced a new section [defaults] that allows setting default values
- Dropped restriction of not setting the same value multiple times,
the last one holds
- Removed a lot of redundant checks that are logically implied or not
really required, without reducing the robustness of the program.
- Improved a lot code readability in some parts, plus added some
useful comments in the parts that were less easy to understand.
- Refactored some functions to be shorter from code lines point of
view, while still doing exactly the same function.
- Various white space and cosmetic code improvements
- Moved macros STR2STR, BOOL2STR, and PM2STR from fpm_conf.h to
fpm_conf.c, no reason to make them public since that's the only file
using them,
=A0 so in case we need to change them in the future, there is less risk
of breaking something which depends on them.
- Fixed a memory leak in fpm_conf_expand_pool_name() (previous
dynamically allocated *value was lost)

Now about the new [defaults] section of the config file.
In the current version if you want to make a decent looking config
file you have to do something like that:

[global]
....your global stuff...

[pool1]
user =3D pool1
listen =3D ...
include =3D pool_defaults.conf

[pool2]
user =3D pool1
listen =3D ...
include =3D pool_defaults.conf

and so on, plus of course you need the external file pool_defaults.conf.

With these changes, you can now do something like this:

[global]
....your global stuff...

[defaults]
....my defaults valid for every pool...
pm.max_children =3D 500
pm.start_servers =3D 10

[pool1]
user =3D pool1
listen =3D ...

[pool2]
user =3D pool2
listen =3D ...

[pool3]
user =3D pool3
listen =3D ...
pm.start_servers =3D 20


There is also a small gain in the parsing time, because defaults are
propagated in memory instead of parsed multiple times as with the
include files solution. I have to admit this argument is quite
irrilevant because it's only a startup time overhead, but it's so much
more elegant in my eyes.

Also by dropping the constraint of setting strings only once, you can
override your defaults in the pools, so you can have an access log
format for all of them except one.


Upcoming changes that would follow from me if you accept this patch:
1) allow '$pool' variable to be used in the [globals], but this
requires a bit of restructuring because it needs to be lazy-expanded
in post process time instead of parsing time.
2) possibility to include more than one file per section


Looking forward to have some feedback from you :)

Kind regards

--=20
Giovanni Giacobbi

--00504502ccb248d31b04a71fa20a--