> 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: > "diagnostics.status_path", "diagnostics.ping_path", > "diagnostics.ping_response". See my feelings on a previous mail.
> - 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. It's a good idea. As the only existent and up to date documentation is in php-fpm.conf, it became very verbose and hard to read, understand and tweak. Maybe we should provide a clean default php-fpm.conf and a full documented version: php-fpm.conf.default and php-fpm.conf.doc. And of course, we should take time to make some real up to date documentation (http://fr.php.net/manual/en/install.fpm.php) > - 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 I've always wanted to do it without taking time to do so. > - 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. +1 on principle but I didn't review the patch deeply. > - Introduced a new section [defaults] that allows setting default values see my comments below > - Dropped restriction of not setting the same value multiple times, > the last one holds see my comments below > - Removed a lot of redundant checks that are logically implied or not > really required, without reducing the robustness of the program. I'm OK with that when it's about optimization of running code (main loop), but if it's for optimizing some init code (before the main loop) , I don't see the benefit as it can prevent bugs when developing new feature or correcting bugs. Double checks at init is not that bad IMO > - Improved a lot code readability in some parts, plus added some > useful comments in the parts that were less easy to understand. +1 on principle but I didn't review the patch deeply. > - Refactored some functions to be shorter from code lines point of > view, while still doing exactly the same function. +1 on principle but I didn't review the patch deeply. > - Various white space and cosmetic code improvements +1 > - 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, > so in case we need to change them in the future, there is less risk > of breaking something which depends on them. I've just commit a patch which does that. (http://svn.php.net/viewvc?view=revision&sortby=date&revision=312922) > - Fixed a memory leak in fpm_conf_expand_pool_name() (previous > dynamically allocated *value was lost) I've commit the fix. (http://svn.php.net/viewvc?view=revision&sortby=date&revision=312923) > > 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 = pool1 > listen = ... > include = pool_defaults.conf > > [pool2] > user = pool1 > listen = ... > include = 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 = 500 > pm.start_servers = 10 > > [pool1] > user = pool1 > listen = ... > > [pool2] > user = pool2 > listen = ... > > [pool3] > user = pool3 > listen = ... > pm.start_servers = 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. the parsing time is not a valid argument here :) But your eyes are (even if I prefer having included files, each and every one of us has its own preferences) > > 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. +1 > > > 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. I don't see where expanding $pool in the [globals] will be used. Only the following parameter can be set in [globals]: pid, error_log, log_level, emergency_restart_threshold, process_control_timeout, daemonize and rlimit* none of them can use the $pool variable. Or I missed something. > 2) possibility to include more than one file per section You can include many file as you want. You can use glob(3) syntax in one include include = /path/to/php-fpm/pools/*.conf or several file in separated includes: include = /path/to/php-fpm/env.conf include = /path/to/php-fpm/log.conf > > > Looking forward to have some feedback from you :) thx you very much for helping us improving PHP and FPM. ++ jerome -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
