On Sun, Nov 7, 2010 at 2:24 PM, Jeff Trawick <[email protected]> wrote:
> On Sun, Nov 7, 2010 at 1:54 PM, <[email protected]> wrote:
>> Author: trawick
>> Date: Sun Nov 7 18:54:44 2010
>> New Revision: 1032345
>>
>> URL: http://svn.apache.org/viewvc?rev=1032345&view=rev
>> Log:
>> mark connection for close after the return from
>> ap_proxy_determine_connection()
>>
>> before this revision: if there was an existing connection,
>> ap_proxy_determine_connection() would close it but then clear
>> the close flag, so it didn't get closed by
>> ap_proxy_release_connection()
>>
>> thus, if this child process doesn't use the connection for a
>> while, the application could be stuck in read() for the same
>> while
>>
>> after: ap_proxy_release_connection() will close it after the
>> request completes
>>
>> Modified:
>> httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
>>
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c?rev=1032345&r1=1032344&r2=1032345&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Sun Nov 7 18:54:44 2010
>> @@ -961,13 +961,6 @@ static int proxy_fcgi_handler(request_re
>>
>> backend->is_ssl = 0;
>>
>> - /* XXX Setting close to 0 is a great way to end up with
>> - * timeouts at this point, since we lack good ways to manage the
>> - * back end fastcgi processes. This should be revisited when we
>> - * have a better story on that part of things. */
>> -
>> - backend->close = 1;
>> -
>> /* Step One: Determine Who To Connect To */
>> status = ap_proxy_determine_connection(p, r, conf, worker, backend,
>> uri, &url, proxyname, proxyport,
>> @@ -977,6 +970,12 @@ static int proxy_fcgi_handler(request_re
>> goto cleanup;
>> }
>>
>> + /* XXX Setting close to 0 is a great way to end up with
>> + * timeouts at this point, since we lack good ways to manage the
>> + * back end fastcgi processes. This should be revisited when we
>> + * have a better story on that part of things. */
>> + backend->close = 1;
>
> is the disablereuse flag more appropriate for indicating what should
> happen with the connection once it is released?
>
> is it practical and a reasonable idea for mod_proxy_fcgi to have a
> different default for disablereuse than other scheme handlers?
>
> hopefully something like this would work:
>
> if (!worker->disablereuse_set) { worker->disablereuse = 1; }
>
> disablereuse could be off for application processes which can handle
> multiple concurrent requests AND can timeout idle connections after an
> interval, but that isn't the typical FastCGI application. For typical
> FastCGI applications, an open connection from one httpd child process
> would prevent it from being able to handle requests from other httpd
> child processes.
I was looking at this for UDS + SetHandler where there are multiple
subtle things preventing connection reuse.
AIUI there are two kinds of things we're talking to -- simple daemons
speaking FCGI (that we're protecting below) that we'd rather queue in
front of with new connections each time, and complex ones like
php-fpm where we probably want the reuse.
Index: modules/proxy/mod_proxy_fcgi.c
===================================================================
--- modules/proxy/mod_proxy_fcgi.c (revision 1646762)
+++ modules/proxy/mod_proxy_fcgi.c (working copy)
@@ -834,11 +834,15 @@
goto cleanup;
}
- /* XXX Setting close to 0 is a great way to end up with
- * timeouts at this point, since we lack good ways to manage the
- * back end fastcgi processes. This should be revisited when we
- * have a better story on that part of things. */
+ /* This scheme handler does not reuse connections by default, to
+ * avoid tieing up a fastcgi that isn't expecting to work on
+ * parallel requests. But if the user went out of their way to
+ * type the default value of disablereuse=off, we'll allow it.
+ */
backend->close = 1;
+ if (worker->s->disablereuse_set && !worker->s->disablereuse) {
+ backend->close = 0;
+ }
Which is e.g.
<Proxy unix:/var/run/php5-fpm.sock|fcgi://localhost>
ProxySet disablereuse=off
</Proxy>
<FilesMatch \.php$>
SetHandler "proxy:unix:/var/run/php5-fpm.sock|fcgi://localhost/"
</FilesMatch>
(With some small patches that allow the UDS worker to be reused)
I tried to copy the sentiment of the email into the comment becuase
for me the email was more clear.
--
Eric Covener
[email protected]
