http://people.apache.org/~covener/patches/proxy-fcgi-uds-reuse.diff

On Fri, Dec 19, 2014 at 11:06 AM, Eric Covener <[email protected]> wrote:
> 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]



-- 
Eric Covener
[email protected]

Reply via email to