You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by el...@apache.org on 2017/07/16 16:15:29 UTC
svn commit: r1802077 - /httpd/httpd/branches/2.4.x/STATUS
Author: elukey
Date: Sun Jul 16 16:15:28 2017
New Revision: 1802077
URL: http://svn.apache.org/viewvc?rev=1802077&view=rev
Log:
Propose backport of r1802040
Modified:
httpd/httpd/branches/2.4.x/STATUS
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1802077&r1=1802076&r2=1802077&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Sun Jul 16 16:15:28 2017
@@ -236,6 +236,10 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
2.4.x patch: http://home.apache.org/~ylavic/patches/httpd-2.4.x-mod_proxy_wstunnel-PR61283.patch
+1: ylavic, jchampion
+ *) mod_proxy_fcgi: Add the support for mod_proxy's flushpackets and flushwait params
+ trunk patch: http://svn.apache.org/r1802040
+ 2.4.x patch: trunk works (modulo CHANGES)
+ +1: elukey
PATCHES/ISSUES THAT ARE BEING WORKED
[ New entries should be added at the START of the list ]
Re: svn commit: r1802077 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Luca Toscano <to...@gmail.com>.
Hi Yann,
2017-07-17 13:49 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
> On Sun, Jul 16, 2017 at 6:15 PM, <el...@apache.org> wrote:
> > Author: elukey
> > Date: Sun Jul 16 16:15:28 2017
> > New Revision: 1802077
> >
> > URL: http://svn.apache.org/viewvc?rev=1802077&view=rev
> > Log:
> > Propose backport of r1802040
> []
> >
> > + *) mod_proxy_fcgi: Add the support for mod_proxy's flushpackets and
> flushwait params
> > + trunk patch: http://svn.apache.org/r1802040
> > + 2.4.x patch: trunk works (modulo CHANGES)
> > + +1: elukey
>
> Not introduced by your change, but about "flushwait" in general: any
> reason why a value of zero defaults to PROXY_FLUSH_WAIT (10ms)?
>
> I think it'd be interesting to let flushwait=0 be a real zero timeout
> (i.e. non-blocking poll() which, on some systems, is likely a more
> efficient syscall than with the current minimal timeout of 1us).
>
> So how about:
> Index: modules/proxy/mod_proxy.c
> ===================================================================
> --- modules/proxy/mod_proxy.c (revision 1802058)
> +++ modules/proxy/mod_proxy.c (working copy)
> @@ -277,10 +277,7 @@ static const char *set_worker_param(apr_pool_t *p,
> if (timeout > 1000000 || timeout < 0) {
> return "flushwait must be <= 1s, or 0 for system default
> of 10 millseconds.";
> }
> - if (timeout == 0)
> - worker->s->flush_wait = PROXY_FLUSH_WAIT;
> - else
> - worker->s->flush_wait = timeout;
> + worker->s->flush_wait = timeout;
> }
> else if (!strcasecmp(key, "ping")) {
> /* Ping/Pong timeout in given unit (default is second).
> ?
>
> PROXY_FLUSH_WAIT would still be the default when no "flushwait=" is
> used (both with flushpackets=on|auto, since this is what we initialize
> flush_wait with by default), but "flushwait=0" would work as
> expected/optimally IMHO.
>
Thanks for bringing this up, it looks strange to me too; +1 from my side.
> It seems that "flushwait=0" is not documented anyway, although the
> real question is: is it used with the current behaviour expected
> (which looks unlikely to me)?
I didn't get anything good out of the svn history/commits, but I'd say
definitely no (at least from the user/admin's point of view).
Luca
Re: svn commit: r1802077 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Jul 16, 2017 at 6:15 PM, <el...@apache.org> wrote:
> Author: elukey
> Date: Sun Jul 16 16:15:28 2017
> New Revision: 1802077
>
> URL: http://svn.apache.org/viewvc?rev=1802077&view=rev
> Log:
> Propose backport of r1802040
[]
>
> + *) mod_proxy_fcgi: Add the support for mod_proxy's flushpackets and flushwait params
> + trunk patch: http://svn.apache.org/r1802040
> + 2.4.x patch: trunk works (modulo CHANGES)
> + +1: elukey
Not introduced by your change, but about "flushwait" in general: any
reason why a value of zero defaults to PROXY_FLUSH_WAIT (10ms)?
I think it'd be interesting to let flushwait=0 be a real zero timeout
(i.e. non-blocking poll() which, on some systems, is likely a more
efficient syscall than with the current minimal timeout of 1us).
So how about:
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c (revision 1802058)
+++ modules/proxy/mod_proxy.c (working copy)
@@ -277,10 +277,7 @@ static const char *set_worker_param(apr_pool_t *p,
if (timeout > 1000000 || timeout < 0) {
return "flushwait must be <= 1s, or 0 for system default
of 10 millseconds.";
}
- if (timeout == 0)
- worker->s->flush_wait = PROXY_FLUSH_WAIT;
- else
- worker->s->flush_wait = timeout;
+ worker->s->flush_wait = timeout;
}
else if (!strcasecmp(key, "ping")) {
/* Ping/Pong timeout in given unit (default is second).
?
PROXY_FLUSH_WAIT would still be the default when no "flushwait=" is
used (both with flushpackets=on|auto, since this is what we initialize
flush_wait with by default), but "flushwait=0" would work as
expected/optimally IMHO.
It seems that "flushwait=0" is not documented anyway, although the
real question is: is it used with the current behaviour expected
(which looks unlikely to me)?
Regards,
Yann.