You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@gmail.com> on 2010/11/07 20:24:00 UTC
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
On Sun, Nov 7, 2010 at 1:54 PM, <tr...@apache.org> 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.
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Posted by Eric Covener <co...@gmail.com>.
On Dec 26, 2014 12:04 PM, "Jim Jagielski" <ji...@jagunet.com> wrote:
> > On Dec 19, 2014, at 10:05 PM, Eric Covener <co...@gmail.com> wrote:
> >
> > On Fri, Dec 19, 2014 at 6:29 PM, Eric Covener <co...@gmail.com> wrote:
> >> http://people.apache.org/~covener/patches/proxy-fcgi-uds-reuse.diff
> >
> > This results in some TCP (but not UDS) slowdown that I don't understand.
>
For all reverse proxys?
I think I debunked it even for fcgi, after adding more threads to php-fpm.
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
For all reverse proxys?
> On Dec 19, 2014, at 10:05 PM, Eric Covener <co...@gmail.com> wrote:
>
> On Fri, Dec 19, 2014 at 6:29 PM, Eric Covener <co...@gmail.com> wrote:
>> http://people.apache.org/~covener/patches/proxy-fcgi-uds-reuse.diff
>
> This results in some TCP (but not UDS) slowdown that I don't understand.
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Posted by Eric Covener <co...@gmail.com>.
On Fri, Dec 19, 2014 at 6:29 PM, Eric Covener <co...@gmail.com> wrote:
> http://people.apache.org/~covener/patches/proxy-fcgi-uds-reuse.diff
This results in some TCP (but not UDS) slowdown that I don't understand.
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Posted by Eric Covener <co...@gmail.com>.
http://people.apache.org/~covener/patches/proxy-fcgi-uds-reuse.diff
On Fri, Dec 19, 2014 at 11:06 AM, Eric Covener <co...@gmail.com> wrote:
> On Sun, Nov 7, 2010 at 2:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>> On Sun, Nov 7, 2010 at 1:54 PM, <tr...@apache.org> 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
> covener@gmail.com
--
Eric Covener
covener@gmail.com
Re: svn commit: r1032345 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Posted by Eric Covener <co...@gmail.com>.
On Sun, Nov 7, 2010 at 2:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Sun, Nov 7, 2010 at 1:54 PM, <tr...@apache.org> 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
covener@gmail.com