RE: [PHP-DEV] [PATCH] PHP_FCGI_TIMEOUT

From: Date: Fri, 03 Dec 2004 06:54:38 +0000
Subject: RE: [PHP-DEV] [PATCH] PHP_FCGI_TIMEOUT
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-13992@lists.php.net to get a copy of this message
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:ada@ardendo.se] Sent: Thursday, December 02, 2004 11:24 To: Andi Gutmans Cc: internals@lists.php.net 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;
+                       }
+                       /* ada@riksnet.se: 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) {
+                       /* ada@riksnet.se: 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


« previous php.internals (#13992) next »