Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:53733
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
In-Reply-To: <CAML1Pf7z1xYL1FGxv9Lb0pq3B=oy0DDSKE3nLQgu2zn-q-Ce-Q@mail.gmail.com>
References: <CAML1Pf7z1xYL1FGxv9Lb0pq3B=oy0DDSKE3nLQgu2zn-q-Ce-Q@mail.gmail.com>
Date: Sun, 3 Jul 2011 15:16:10 +0200
Message-ID: <CAML1Pf4-TChFg7khaJuNmtwO55sQgBfWeCFt2RpiTHN=mnqm6w@mail.gmail.com>
To: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: Big patch for FPM (config and initialization)
From: giovanni@giacobbi.net (Giovanni Giacobbi)

Sorry I always forget this mailing list is "special" and strip my attachmen=
ts...

Here is a link:
http://pastebin.com/7JpXr22c

On Sun, Jul 3, 2011 at 02:50, Giovanni Giacobbi <giovanni@giacobbi.net> wro=
te:
> 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
>
> --
> Giovanni Giacobbi
>



--=20
Giovanni Giacobbi