Agreed. I'm against applying this patch.
At 09:13 AM 12/3/2004 +0300, Dmitry Stogov wrote:
libfcgi that is used by PHP CGI SAPI uses SIGUSR1 for graceful restart. This feature are used by some FastCGI front-ends, mod_fastcgi for example (see notes section on http://www.fastcgi.com/mod_fastcgi/docs/mod_fastcgi.html) Your patch breaks this.
Also it can break scripts that require more then PHP_FCGI_TIMEOUT time for execution.
At least, FastCGI front-ends have ability to kill dynamic servers by himself, so why we need to implement this functionality in PHP?
Thanks. Dmitry.
> -----Original Message----- > From: Christer Holgersson [mailto:[EMAIL PROTECTED] > Sent: Thursday, December 02, 2004 11:24 > To: Andi Gutmans > Cc: [EMAIL PROTECTED] > Subject: Re: [PHP-DEV] [PATCH] PHP_FCGI_TIMEOUT > > > > Currently apache and lighttpd. Both are feasible to patch, > but I looked into adding this functionality to lighttpd, and > that would require a more complicated patch than this, so this won ;-) > > As you said, it is not the end of the world. I am quite happy > to patch PHP myself when needed, but thought I'd share this > in case someone else has the same needs. If you do not want > it enabled in the mainline version maybe making it "#ifdef > PHP_FCGI_TIMEOUT" > would be an idea? > > Cheers, > /C > > > On Thu, Dec 02, 2004 at 12:08:37AM -0800, Andi Gutmans wrote: > > What front-end are you using? Look it's not the end of the > world but > > you're > > the first person who seems to have wanted this and I don't > see dozens of > > front-ends being patched to support this. I think it's > quite esoteric and I > > don't like seeing more and more patches, even if they don't > do anything > > under normal circumstances, if they aren't clearly advantageous. > > Anyway, maybe talk a bit more about what front-ends you see > this being used > > with and so on and maybe the need will be clearer. > > Thanks. > > > > At 09:04 AM 12/2/2004 +0100, Christer Holgersson wrote: > > > > >Yes, it could be implemented that way too, but it would require a > > >patch to all the different front-ends you might be using > instead of > > >one simple patch to the backend. You might also have a > front-end that > > >you do not have the source for, and thus cannot patch. > > > > > >I consider it a worthwhile patch. It is extremely simple, and does > > >not do anyhing unless the PHP_FCGI_TIMEOUT environment variable is > > >set. Summary: it is simply much easier to add this > functionality here > > >than anywhere else as I see it. > > > > > >Cheers, > > > /Chris > > > > > >On Wed, Dec 01, 2004 at 11:35:17PM -0800, Andi Gutmans wrote: > > >> I must be missing something. Again, why shouldn't this > be the job > > >> of whatever is running the FastCGI stuff? So what if it's a web > > >> frontend? It should be able to kill the mother implementation so > > >> that it also kills > > >its > > >> children. > > >> > > >> At 08:27 AM 12/2/2004 +0100, Christer Holgersson wrote: > > >> > > >> >If you are running the FastCGI version of PHP it will > start up a > > >> >number (default 8) of "child" PHP interpretors that will wait > > >> >around indefinitely for work to do, and the "mother" FastCGI > > >> >process will automatically restart any process that terminates > > >> >(and automatically restart every child after a default of 500 > > >> >requests). > > >> > > > >> >If you are using a web frontend that dynamically starts the > > >> >FastCGI processes when needed it is totally unnecessary for the > > >> >FastCGI:s to hang around when there is no demand for them, thus > > >> >this patch. It will terminate the mother FastCGI and > all children > > >> >after PHP_FASTCGI_TIMEOUT seconds with no request to any of the > > >> >children, and whenever there is demand for the FastCGI > again the > > >> >web frontend will restart the processes. > > >> > > > >> >This is excellent if you have a lot of scripts that are seldom > > >> >invoked, but that you want to run as FastCGI:s when there is > > >> >actual demand for them, and that are chroot:ed so that > you have a > > >> >diffent FastCGI PHP started for each of them. In this case you > > >> >_do_ want them to self-termninate when there is no demand. > > >> > > > >> >Cheers, > > >> > /C > > >> > > > >> >On Wed, Dec 01, 2004 at 04:18:37PM -0800, Andi Gutmans wrote: > > >> >> I don't see why this patch is needed. It's the FastCGI > > >implementation's > > >> >job > > >> >> to do the right thing and terminate the processes > according to > > >> >> some > > >> >kind of > > >> >> strategy. The FCGI processes shouldn't self destruct. > > >> >> > > >> >> Andi > > >> >> > > >> >> At 07:11 PM 12/1/2004 +0100, Christer Holgersson wrote: > > >> >> > > >> >> >Included is a patch that adds a PHP_FCGI_TIMEOUT that works > > >> >> >somewhat akin to PHP_FCGI_MAX_REQUESTS. With > PHP_FCGI_TIMEOUT > > >> >> >set to 300 the whole fcgi process will terminate > after 300 sec > > >> >> >with no request to any of the worker children - > great for those > > >> >> >seldom used dynamically created fcgi scripts. > > >> >> > > > >> >> >Cheers, > > >> >> > /Chris > > >> >> > > > >> >> >PS. The patch works just as well in 5.0.2 as in 4.3.9. > > >> >> > > > >> >> > > > >> >> >diff -rub php-4.3.9/sapi/cgi/cgi_main.c > > >> >php-4.3.9-ada/sapi/cgi/cgi_main.c > > >> >> >--- php-4.3.9/sapi/cgi/cgi_main.c 2004-07-15 > 00:38:18.000000000 > > >> >+0200 > > >> >> >+++ php-4.3.9-ada/sapi/cgi/cgi_main.c 2004-12-01 > 09:00:58.595259999 > > >> >+0100 > > >> >> >@@ -946,6 +946,24 @@ > > >> >> > /* We should exit at this point, but MacOSX doesn't > > >> >> >seem to > > >*/ > > >> >> > exit( 0 ); > > >> >> > } > > >> >> >+ > > >> >> >+ > > >> >> >+static int php_fcgi_timeout = 0; > > >> >> >+ > > >> >> >+static void reset_timeout() > > >> >> >+{ > > >> >> >+ alarm(php_fcgi_timeout); > > >> >> >+ /* fprintf(stderr, "FastCGI: reset timeout\n"); */ > > >> >> >+ signal(SIGUSR1, reset_timeout); > > >> >> >+} > > >> >> >+ > > >> >> >+static void handle_timeout() > > >> >> >+{ > > >> >> >+ /* fprintf(stderr, "FastCGI: timeout\n"); */ > > >> >> >+ fastcgi_cleanup(SIGALRM); > > >> >> >+ exit(0); > > >> >> >+} > > >> >> >+ > > >> >> > #endif > > >> >> > > > >> >> > /* {{{ main > > >> >> >@@ -980,6 +998,7 @@ > > >> >> > #endif > > >> >> > > > >> >> > #if PHP_FASTCGI > > >> >> >+ int php_fcgi_pid = getpid(); > > >> >> > int max_requests = 500; > > >> >> > int requests = 0; > > >> >> > int fastcgi = !FCGX_IsCGI(); > > >> >> >@@ -1178,6 +1197,20 @@ > > >> >> > } > > >> >> > } > > >> >> > > > >> >> >+ /* How long all children can be idle before > > >terminating > > >> >*/ > > >> >> >+ if( getenv( "PHP_FCGI_TIMEOUT" )) { > > >> >> >+ php_fcgi_timeout = atoi( getenv( > > >> >> >"PHP_FCGI_TIMEOUT" )); > > >> >> >+ if( !php_fcgi_timeout ) { > > >> >> >+ fprintf( stderr, > > >> >> >+ "PHP_FCGI_TIMEOUT is > > >> >> >+ not > > >> >valid\n" > > >> >> >); > > >> >> >+ return FAILURE; > > >> >> >+ } > > >> >> >+ /* [EMAIL PROTECTED]: children signal > > >> >> >+ parent > > >when > > >> >> >they do work, */ > > >> >> >+ /* which resets the > timeout value */ > > >> >> >+ signal(SIGALRM, handle_timeout); > > >> >> >+ signal(SIGUSR1, reset_timeout); > > >> >> >+ } > > >> >> >+ > > >> >> > /* make php call us to get _ENV vars */ > > >> >> > php_php_import_environment_variables = > > >> >> >php_import_environment_variables; > > >> >> > php_import_environment_variables = > > >> >> >cgi_php_import_environment_variables; > > >> >> >@@ -1305,6 +1338,8 @@ > > >> >> > > > >> >> > while (!fastcgi > > >> >> > || FCGX_Accept_r( &request ) >= 0) { > > >> >> >+ /* [EMAIL PROTECTED]: let > parent know we > > >> >> >+ have > > >> >work to > > >> >> >do */ > > >> >> >+ if (php_fcgi_timeout) > > >> >> >+ kill(php_fcgi_pid, > > >> >SIGUSR1); > > >> >> > #endif > > >> >> > > > >> >> > #if PHP_FASTCGI > > >> >> > > > >> >> >-- > > >> >> >PHP Internals - PHP Runtime Development Mailing List To > > >> >> >unsubscribe, visit: http://www.php.net/unsub.php > > >> >> > > > > -- > Christer Holgersson > CTO, Ardendo AB > www.ardendo.se > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
