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

Reply via email to