You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Fritsch <sf...@sfritsch.de> on 2011/11/15 18:17:33 UTC
Re: svn commit: r1202255 -
/httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
On Tue, 15 Nov 2011, pquerna@apache.org wrote:
> Author: pquerna
> Date: Tue Nov 15 15:49:19 2011
> New Revision: 1202255
>
> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
> Log:
> disable mod_reqtimeout if not configured
Why that? We have just changed the default to be enabled in r1199447 and
several developers at the hackathon agreed to this change.
>
> Modified:
> httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
>
> Modified: httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_reqtimeout.c?rev=1202255&r1=1202254&r2=1202255&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_reqtimeout.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_reqtimeout.c Tue Nov 15 15:49:19 2011
> @@ -340,6 +340,11 @@ static int reqtimeout_init(conn_rec *c)
> return DECLINED;
> }
>
> + if (cfg->header_timeout == UNSET && cfg->body_timeout == UNSET) {
> + /* if everything is unset, skip by default. */
> + return DECLINED;
> + }
> +
> ccfg = apr_pcalloc(c->pool, sizeof(reqtimeout_con_cfg));
> ccfg->type = "header";
> if (cfg->header_timeout != UNSET) {
>
>
>
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 16 November 2011, William A. Rowe Jr. wrote:
> Were these results all based on 100% cpu pegged?
Yes, the machine running httpd had both cores at 100%. The machine
running ab had a single core at ~ 70%.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
Were these results all based on 100% cpu pegged?
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 16 November 2011, Stefan Fritsch wrote:
> > What I am really opposed to is that the LoadModule causes such a
> > degradation in performance.
>
> In my quick (and maybe not that accurate) tests, the penalty caused
> by mod_reqtimeout (1.4%) was smaller than the penalty of not
> having BufferedLogs on (2.1%). But there is no question that the
> current default 'BufferedLogs off' is better for the majority of
> users, just the same as having reqtimeout enabled is better for
> the majority of users. Actually, RequestReadTimeout should
> probably be into the core, but none got around to moving it.
I did some more accurate benchmarks on a dual core AMD E350, which is
slow enough that it can be saturated with a simple ab run from a fast
machine over a 1GB/s link. The numbers are reproducible over several
runs with less than 0.5% variation.
Requesting a zero-size file
ab -c 100 -n 100000 http://10.1.1.20:8081/zero
Trunk r1202537
Default configure, but CFLAGS=-O3
Default config, but
StartServers 2
MinSpareThreads 20
MaxSpareThreads 100
ThreadLimit 200
ThreadsPerChild 50
MaxRequestWorkers 600
The reqtimeout config was
RequestReadTimeout header=20-40,MinRate=500 body=20,MinRate=500
if enabled
Results:
reqtimeout disabled, buffered logs off 4060 100%
mod_reqtimeout not loaded, buffered logs off 4090 100.7%
reqtimeout disabled, buffered logs on 4378 107.8%
reqtimeout enabled, buffered logs on 4299 105.9%
reqtimeout enabled, buffered logs off 3934 96.9%
reqtimeout enabled+r1202886, buffered logs off 3996 98.5%
Even without r1202886, I think the degradation would have been
acceptable. With r1202886 it's even less of a problem.
To put the number into perspective, if I request the default 45 byte
index.html instead, the request rate drops by approx. 16%.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 16 November 2011, Paul Querna wrote:
> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung
<ra...@kippdata.de> wrote:
> > On 15.11.2011 20:57, Jeff Trawick wrote:
> >> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
> >>
> >> <wr...@rowe-clan.net> wrote:
> >>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
> >>>> On Tuesday 15 November 2011, Paul Querna wrote:
> >>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan
> >>>>> Fritsch<sf...@sfritsch.de>
> >>>>
> >>>> wrote:
> >>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
> >>>>>>> Author: pquerna
> >>>>>>> Date: Tue Nov 15 15:49:19 2011
> >>>>>>> New Revision: 1202255
> >>>>>>>
> >>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
> >>>>>>> Log:
> >>>>>>> disable mod_reqtimeout if not configured
> >>>>>>
> >>>>>> Why that? We have just changed the default to be enabled in
> >>>>>> r1199447 and several developers at the hackathon agreed to
> >>>>>> this change.
> >>>>>
> >>>>> Didn't know it was discussed in depth at the hackathon, and
> >>>>> there wasn't any discussion on the list....
> >>>>>
> >>>>> It showed up quite quickly in my profiling of the Event MPM,
> >>>>> because every pull/push on the filters would cause a
> >>>>> apr_time_now() call.
> >>>>>
> >>>>> I don't really like that just by loading the module, it
> >>>>> changes the behavior and performance of the server so
> >>>>> drastically.
> >>>>
> >>>> It only acts on reads from the client. Normal non-POST
> >>>> requests arrive in one or two packets, which would mean
> >>>> approx. 3 additional apr_time_now calls per request. I
> >>>> haven't done benchmarks, but I can't imagine that this has a
> >>>> drastic impact on performance. And if it costs 1-2%, then
> >>>> that's a small cost compared to the impact of slowloris type
> >>>> attacks which eat lots of memory.
> >>>>
> >>>> The general intention of the recent changes in default configs
> >>>> and module selection/loading was to make it easier to only
> >>>> load those modules that are really needed, have a reasonable
> >>>> default config, and have the compiled-in default values be
> >>>> the same as those in the example config files.
> >>>
> >>> Which means, build by default, disable by default. I think
> >>> that keeps everyone happy. When abuse arrives, it's trivial
> >>> to load.
> >>
> >> Timeout 60 isn't nearly as bad as the old Timeout 300 that is
> >> probably still in wide use, but mod_reqtimeout can provide a
> >> much more reasonable out of the box configuration. I think we
> >> should keep it in place by default.
> >
> > +1
>
> What I am really opposed to is that the LoadModule causes such a
> degradation in performance.
In my quick (and maybe not that accurate) tests, the penalty caused by
mod_reqtimeout (1.4%) was smaller than the penalty of not having
BufferedLogs on (2.1%). But there is no question that the current
default 'BufferedLogs off' is better for the majority of users, just
the same as having reqtimeout enabled is better for the majority of
users. Actually, RequestReadTimeout should probably be into the core,
but none got around to moving it.
> I am 100% +1 to adding conf commands to the default configuration
> in the httpd.conf, but what I do not like is that having just a
> LoadModule with nothing else causes reqtimeout to do work. It is
> too trivial for people to have accidental load modules in many
> distros.
There are other cases where just loading a module can have much larger
influence on the performance (look at AP_AUTH_INTERNAL_PER_URI versus
AP_AUTH_INTERNAL_PER_CONF in the auth hooks). We need to educate users
that they should disable modules that they don't need.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/16/2011 5:06 PM, Stefan Fritsch wrote:
> On Wednesday 16 November 2011, William A. Rowe Jr. wrote:
>>
>> Will people be confused by a directive that does nothing when
>> placed in a named vhost? Or should this just be global-only and
>> forget about it? It would be nice to cripple this module, say,
>> for the intranet side of a server, and enable the overhead only on
>> the external side.
>
> Currently, it's per phys-vhost. Many SSL directives also have no
> effect in non-default named-vhost, therefore I don't think that
> RequestReadTimeout makes things worse.
It would really be amazing if we could distinguish the current
vhost context as physical (al la first-of-many named) or a named
host (being at least the second instance).
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 16 November 2011, William A. Rowe Jr. wrote:
> On 11/16/2011 10:20 AM, Paul Querna wrote:
> > I am 100% +1 to adding conf commands to the default configuration
> > in the httpd.conf, but what I do not like is that having just a
> > LoadModule with nothing else causes reqtimeout to do work. It
> > is too trivial for people to have accidental load modules in
> > many distros.
>
> +1; in this case it will be tricky. This would be a per listener
> or per phys-vhost option. Although it could be a per-named-vhost
> option, it makes next to no sense during the request headers input
> phase when no host: will be parsed.
>
> Will people be confused by a directive that does nothing when
> placed in a named vhost? Or should this just be global-only and
> forget about it? It would be nice to cripple this module, say,
> for the intranet side of a server, and enable the overhead only on
> the external side.
Currently, it's per phys-vhost. Many SSL directives also have no
effect in non-default named-vhost, therefore I don't think that
RequestReadTimeout makes things worse.
> There's also nothing wrong with having it default to enabled,
> provided the admin can disable it most of the time if they choose,
> for speed.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/16/2011 10:20 AM, Paul Querna wrote:
>
> I am 100% +1 to adding conf commands to the default configuration in
> the httpd.conf, but what I do not like is that having just a
> LoadModule with nothing else causes reqtimeout to do work. It is too
> trivial for people to have accidental load modules in many distros.
+1; in this case it will be tricky. This would be a per listener
or per phys-vhost option. Although it could be a per-named-vhost
option, it makes next to no sense during the request headers input
phase when no host: will be parsed.
Will people be confused by a directive that does nothing when placed
in a named vhost? Or should this just be global-only and forget
about it? It would be nice to cripple this module, say, for the
intranet side of a server, and enable the overhead only on the
external side.
There's also nothing wrong with having it default to enabled, provided
the admin can disable it most of the time if they choose, for speed.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, Nov 17, 2011 at 8:22 AM, Jeff Trawick <tr...@gmail.com> wrote:
> On Thu, Nov 17, 2011 at 8:00 AM, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>> On Nov 17, 2011, at 7:32 AM, Jeff Trawick wrote:
>>>
>>> FWIW, using the optimized rough time from the Event listener thread
>>> within mod_reqtimeout is consistently faster in my testing.
>>
>> Same here...
>
> Thanks for trying it out; my individual runs of a particular testcase
> were not consistent enough to get abundant warm fuzzies but it did
> look good considering the ranges of runs with/without the change.
>
> In the next couple of days I'll try to actually *read* the listener
> thread code (to see if the optimization needs to be disabled during
> certain intervals, like shutdown) and clean up/commit the API to
> trunk. As long as it remains an API that works even if the MPM
> doesn't/can't-practically provide an optimized version, it can go into
> 2.4.x at any point.
There's also the atomicity of load/store in the event implementation
to consider... 64-bit apr_time_t, no portable 64-bit atomics in
APR... it could be bad news if a caller sees the new high-order
32-bits and the old low-order 32-bits...
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, Nov 17, 2011 at 8:00 AM, Jim Jagielski <ji...@jagunet.com> wrote:
>
> On Nov 17, 2011, at 7:32 AM, Jeff Trawick wrote:
>>
>> FWIW, using the optimized rough time from the Event listener thread
>> within mod_reqtimeout is consistently faster in my testing.
>
> Same here...
Thanks for trying it out; my individual runs of a particular testcase
were not consistent enough to get abundant warm fuzzies but it did
look good considering the ranges of runs with/without the change.
In the next couple of days I'll try to actually *read* the listener
thread code (to see if the optimization needs to be disabled during
certain intervals, like shutdown) and clean up/commit the API to
trunk. As long as it remains an API that works even if the MPM
doesn't/can't-practically provide an optimized version, it can go into
2.4.x at any point.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Nov 17, 2011, at 7:32 AM, Jeff Trawick wrote:
>
> FWIW, using the optimized rough time from the Event listener thread
> within mod_reqtimeout is consistently faster in my testing.
Same here...
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Nov 16, 2011 at 2:26 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 2:23 PM, Jeff Trawick <tr...@gmail.com> wrote:
>> On Wed, Nov 16, 2011 at 11:51 AM, Jeff Trawick <tr...@gmail.com> wrote:
>>> On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <pa...@querna.org> wrote:
>>>> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <ra...@kippdata.de> wrote:
>>>>> On 15.11.2011 20:57, Jeff Trawick wrote:
>>>>>>
>>>>>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
>>>>>> <wr...@rowe-clan.net> wrote:
>>>>>>>
>>>>>>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>>>>>>
>>>>>>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>>>>>>
>>>>>>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>>>>>>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>>>>>>
>>>>>>>>>>> Author: pquerna
>>>>>>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>>>>>>> New Revision: 1202255
>>>>>>>>>>>
>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>>>>>>> Log:
>>>>>>>>>>> disable mod_reqtimeout if not configured
>>>>>>>>>>
>>>>>>>>>> Why that? We have just changed the default to be enabled in
>>>>>>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>>>>>>> change.
>>>>>>>>>
>>>>>>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>>>>>>> wasn't any discussion on the list....
>>>>>>>>>
>>>>>>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>>>>>>> because every pull/push on the filters would cause a
>>>>>>>>> apr_time_now() call.
>>>>>>>>>
>>>>>>>>> I don't really like that just by loading the module, it changes the
>>>>>>>>> behavior and performance of the server so drastically.
>>>>>>>>
>>>>>>>> It only acts on reads from the client. Normal non-POST requests arrive
>>>>>>>> in one or two packets, which would mean approx. 3 additional
>>>>>>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>>>>>>> imagine that this has a drastic impact on performance. And if it costs
>>>>>>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>>>>>>> type attacks which eat lots of memory.
>>>>>>>>
>>>>>>>> The general intention of the recent changes in default configs and
>>>>>>>> module selection/loading was to make it easier to only load those
>>>>>>>> modules that are really needed, have a reasonable default config, and
>>>>>>>> have the compiled-in default values be the same as those in the
>>>>>>>> example config files.
>>>>>>>
>>>>>>> Which means, build by default, disable by default. I think that keeps
>>>>>>> everyone happy. When abuse arrives, it's trivial to load.
>>>>>>
>>>>>> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
>>>>>> still in wide use, but mod_reqtimeout can provide a much more
>>>>>> reasonable out of the box configuration. I think we should keep it in
>>>>>> place by default.
>>>>>
>>>>> +1
>>>>>
>>>>
>>>> What I am really opposed to is that the LoadModule causes such a
>>>> degradation in performance.
>>>>
>>>> I am 100% +1 to adding conf commands to the default configuration in
>>>> the httpd.conf, but what I do not like is that having just a
>>>> LoadModule with nothing else causes reqtimeout to do work. It is too
>>>> trivial for people to have accidental load modules in many distros.
>>>
>>> How many distinct concerns are there? (Cut through the detail that
>>> the implementation of request phase-specific timeouts is in a module,
>>> and hence conflicts with thoughts for what it means to have code
>>> loaded by LoadModule but not explicitly configured.) I see at least
>>> two raised.
>>>
>>> 1) performance degradation out of the box with no explicit configuration*
>>> 2) any processing at all out of the box with no explicit configuration
>>> 3) relatively complex-to-understand timeout mechanism out of the box
>>> with no explicit configuration (independent of whether it is core or
>>> mod_)
>>> 4) disagreement that the mod_reqtimeout defaults are "generally
>>> better" across arbitrary situations than Timeout <somenumber>
>>> 5) ???
>>>
>>> (I assume that the performance issue related to the apr_time_now()
>>> calls in mod_reqtimeout is addressable, and probably in a way that
>>> helps other modules that don't need to know the time with high
>>> precision.)
>>
>> See attached patch for one possible fix (completely untested :) ). It
>> should be determined (even if by changing code) that event's listener
>> thread can update the timestamp in the child at least every 200ms.
>> ap_time_around_now(), ap_roughly_time_now(), etc. might be better API
>> names. Over the long term it might be helpful to have a couple of
>> versions with different requirements, as some MPMs will simply punt to
>> apr_time_now() if asked to provide something within 200ms.
>
> oops, I forgot to 0 out roughly_now during special situations (maybe
> just exiting?); its just a concept patch ;)
FWIW, using the optimized rough time from the Event listener thread
within mod_reqtimeout is consistently faster in my testing. With a
-DAPPROXIMATE_TIME command-line parm it could be used for the time of
the request as well.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Nov 16, 2011 at 2:23 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 11:51 AM, Jeff Trawick <tr...@gmail.com> wrote:
>> On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <pa...@querna.org> wrote:
>>> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <ra...@kippdata.de> wrote:
>>>> On 15.11.2011 20:57, Jeff Trawick wrote:
>>>>>
>>>>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
>>>>> <wr...@rowe-clan.net> wrote:
>>>>>>
>>>>>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>>>>>
>>>>>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>>>>>
>>>>>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>>>>>
>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>>>>>
>>>>>>>>>> Author: pquerna
>>>>>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>>>>>> New Revision: 1202255
>>>>>>>>>>
>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>>>>>> Log:
>>>>>>>>>> disable mod_reqtimeout if not configured
>>>>>>>>>
>>>>>>>>> Why that? We have just changed the default to be enabled in
>>>>>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>>>>>> change.
>>>>>>>>
>>>>>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>>>>>> wasn't any discussion on the list....
>>>>>>>>
>>>>>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>>>>>> because every pull/push on the filters would cause a
>>>>>>>> apr_time_now() call.
>>>>>>>>
>>>>>>>> I don't really like that just by loading the module, it changes the
>>>>>>>> behavior and performance of the server so drastically.
>>>>>>>
>>>>>>> It only acts on reads from the client. Normal non-POST requests arrive
>>>>>>> in one or two packets, which would mean approx. 3 additional
>>>>>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>>>>>> imagine that this has a drastic impact on performance. And if it costs
>>>>>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>>>>>> type attacks which eat lots of memory.
>>>>>>>
>>>>>>> The general intention of the recent changes in default configs and
>>>>>>> module selection/loading was to make it easier to only load those
>>>>>>> modules that are really needed, have a reasonable default config, and
>>>>>>> have the compiled-in default values be the same as those in the
>>>>>>> example config files.
>>>>>>
>>>>>> Which means, build by default, disable by default. I think that keeps
>>>>>> everyone happy. When abuse arrives, it's trivial to load.
>>>>>
>>>>> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
>>>>> still in wide use, but mod_reqtimeout can provide a much more
>>>>> reasonable out of the box configuration. I think we should keep it in
>>>>> place by default.
>>>>
>>>> +1
>>>>
>>>
>>> What I am really opposed to is that the LoadModule causes such a
>>> degradation in performance.
>>>
>>> I am 100% +1 to adding conf commands to the default configuration in
>>> the httpd.conf, but what I do not like is that having just a
>>> LoadModule with nothing else causes reqtimeout to do work. It is too
>>> trivial for people to have accidental load modules in many distros.
>>
>> How many distinct concerns are there? (Cut through the detail that
>> the implementation of request phase-specific timeouts is in a module,
>> and hence conflicts with thoughts for what it means to have code
>> loaded by LoadModule but not explicitly configured.) I see at least
>> two raised.
>>
>> 1) performance degradation out of the box with no explicit configuration*
>> 2) any processing at all out of the box with no explicit configuration
>> 3) relatively complex-to-understand timeout mechanism out of the box
>> with no explicit configuration (independent of whether it is core or
>> mod_)
>> 4) disagreement that the mod_reqtimeout defaults are "generally
>> better" across arbitrary situations than Timeout <somenumber>
>> 5) ???
>>
>> (I assume that the performance issue related to the apr_time_now()
>> calls in mod_reqtimeout is addressable, and probably in a way that
>> helps other modules that don't need to know the time with high
>> precision.)
>
> See attached patch for one possible fix (completely untested :) ). It
> should be determined (even if by changing code) that event's listener
> thread can update the timestamp in the child at least every 200ms.
> ap_time_around_now(), ap_roughly_time_now(), etc. might be better API
> names. Over the long term it might be helpful to have a couple of
> versions with different requirements, as some MPMs will simply punt to
> apr_time_now() if asked to provide something within 200ms.
oops, I forgot to 0 out roughly_now during special situations (maybe
just exiting?); its just a concept patch ;)
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Nov 16, 2011 at 11:51 AM, Jeff Trawick <tr...@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <pa...@querna.org> wrote:
>> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <ra...@kippdata.de> wrote:
>>> On 15.11.2011 20:57, Jeff Trawick wrote:
>>>>
>>>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
>>>> <wr...@rowe-clan.net> wrote:
>>>>>
>>>>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>>>>
>>>>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>>>>
>>>>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>>>>
>>>>>> wrote:
>>>>>>>>
>>>>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>>>>
>>>>>>>>> Author: pquerna
>>>>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>>>>> New Revision: 1202255
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>>>>> Log:
>>>>>>>>> disable mod_reqtimeout if not configured
>>>>>>>>
>>>>>>>> Why that? We have just changed the default to be enabled in
>>>>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>>>>> change.
>>>>>>>
>>>>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>>>>> wasn't any discussion on the list....
>>>>>>>
>>>>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>>>>> because every pull/push on the filters would cause a
>>>>>>> apr_time_now() call.
>>>>>>>
>>>>>>> I don't really like that just by loading the module, it changes the
>>>>>>> behavior and performance of the server so drastically.
>>>>>>
>>>>>> It only acts on reads from the client. Normal non-POST requests arrive
>>>>>> in one or two packets, which would mean approx. 3 additional
>>>>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>>>>> imagine that this has a drastic impact on performance. And if it costs
>>>>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>>>>> type attacks which eat lots of memory.
>>>>>>
>>>>>> The general intention of the recent changes in default configs and
>>>>>> module selection/loading was to make it easier to only load those
>>>>>> modules that are really needed, have a reasonable default config, and
>>>>>> have the compiled-in default values be the same as those in the
>>>>>> example config files.
>>>>>
>>>>> Which means, build by default, disable by default. I think that keeps
>>>>> everyone happy. When abuse arrives, it's trivial to load.
>>>>
>>>> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
>>>> still in wide use, but mod_reqtimeout can provide a much more
>>>> reasonable out of the box configuration. I think we should keep it in
>>>> place by default.
>>>
>>> +1
>>>
>>
>> What I am really opposed to is that the LoadModule causes such a
>> degradation in performance.
>>
>> I am 100% +1 to adding conf commands to the default configuration in
>> the httpd.conf, but what I do not like is that having just a
>> LoadModule with nothing else causes reqtimeout to do work. It is too
>> trivial for people to have accidental load modules in many distros.
>
> How many distinct concerns are there? (Cut through the detail that
> the implementation of request phase-specific timeouts is in a module,
> and hence conflicts with thoughts for what it means to have code
> loaded by LoadModule but not explicitly configured.) I see at least
> two raised.
>
> 1) performance degradation out of the box with no explicit configuration*
> 2) any processing at all out of the box with no explicit configuration
> 3) relatively complex-to-understand timeout mechanism out of the box
> with no explicit configuration (independent of whether it is core or
> mod_)
> 4) disagreement that the mod_reqtimeout defaults are "generally
> better" across arbitrary situations than Timeout <somenumber>
> 5) ???
>
> (I assume that the performance issue related to the apr_time_now()
> calls in mod_reqtimeout is addressable, and probably in a way that
> helps other modules that don't need to know the time with high
> precision.)
See attached patch for one possible fix (completely untested :) ). It
should be determined (even if by changing code) that event's listener
thread can update the timestamp in the child at least every 200ms.
ap_time_around_now(), ap_roughly_time_now(), etc. might be better API
names. Over the long term it might be helpful to have a couple of
versions with different requirements, as some MPMs will simply punt to
apr_time_now() if asked to provide something within 200ms.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <pa...@querna.org> wrote:
> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <ra...@kippdata.de> wrote:
>> On 15.11.2011 20:57, Jeff Trawick wrote:
>>>
>>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
>>> <wr...@rowe-clan.net> wrote:
>>>>
>>>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>>>
>>>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>>>
>>>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>>>
>>>>> wrote:
>>>>>>>
>>>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>>>
>>>>>>>> Author: pquerna
>>>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>>>> New Revision: 1202255
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>>>> Log:
>>>>>>>> disable mod_reqtimeout if not configured
>>>>>>>
>>>>>>> Why that? We have just changed the default to be enabled in
>>>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>>>> change.
>>>>>>
>>>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>>>> wasn't any discussion on the list....
>>>>>>
>>>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>>>> because every pull/push on the filters would cause a
>>>>>> apr_time_now() call.
>>>>>>
>>>>>> I don't really like that just by loading the module, it changes the
>>>>>> behavior and performance of the server so drastically.
>>>>>
>>>>> It only acts on reads from the client. Normal non-POST requests arrive
>>>>> in one or two packets, which would mean approx. 3 additional
>>>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>>>> imagine that this has a drastic impact on performance. And if it costs
>>>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>>>> type attacks which eat lots of memory.
>>>>>
>>>>> The general intention of the recent changes in default configs and
>>>>> module selection/loading was to make it easier to only load those
>>>>> modules that are really needed, have a reasonable default config, and
>>>>> have the compiled-in default values be the same as those in the
>>>>> example config files.
>>>>
>>>> Which means, build by default, disable by default. I think that keeps
>>>> everyone happy. When abuse arrives, it's trivial to load.
>>>
>>> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
>>> still in wide use, but mod_reqtimeout can provide a much more
>>> reasonable out of the box configuration. I think we should keep it in
>>> place by default.
>>
>> +1
>>
>
> What I am really opposed to is that the LoadModule causes such a
> degradation in performance.
>
> I am 100% +1 to adding conf commands to the default configuration in
> the httpd.conf, but what I do not like is that having just a
> LoadModule with nothing else causes reqtimeout to do work. It is too
> trivial for people to have accidental load modules in many distros.
How many distinct concerns are there? (Cut through the detail that
the implementation of request phase-specific timeouts is in a module,
and hence conflicts with thoughts for what it means to have code
loaded by LoadModule but not explicitly configured.) I see at least
two raised.
1) performance degradation out of the box with no explicit configuration*
2) any processing at all out of the box with no explicit configuration
3) relatively complex-to-understand timeout mechanism out of the box
with no explicit configuration (independent of whether it is core or
mod_)
4) disagreement that the mod_reqtimeout defaults are "generally
better" across arbitrary situations than Timeout <somenumber>
5) ???
(I assume that the performance issue related to the apr_time_now()
calls in mod_reqtimeout is addressable, and probably in a way that
helps other modules that don't need to know the time with high
precision.)
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Nov 16, 2011, at 11:20 AM, Paul Querna wrote:
>
> What I am really opposed to is that the LoadModule causes such a
> degradation in performance.
>
> I am 100% +1 to adding conf commands to the default configuration in
> the httpd.conf, but what I do not like is that having just a
> LoadModule with nothing else causes reqtimeout to do work. It is too
> trivial for people to have accidental load modules in many distros.
>
+1
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Paul Querna <pa...@querna.org>.
On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <ra...@kippdata.de> wrote:
> On 15.11.2011 20:57, Jeff Trawick wrote:
>>
>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
>> <wr...@rowe-clan.net> wrote:
>>>
>>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>>
>>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>>
>>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>>
>>>> wrote:
>>>>>>
>>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>>
>>>>>>> Author: pquerna
>>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>>> New Revision: 1202255
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>>> Log:
>>>>>>> disable mod_reqtimeout if not configured
>>>>>>
>>>>>> Why that? We have just changed the default to be enabled in
>>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>>> change.
>>>>>
>>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>>> wasn't any discussion on the list....
>>>>>
>>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>>> because every pull/push on the filters would cause a
>>>>> apr_time_now() call.
>>>>>
>>>>> I don't really like that just by loading the module, it changes the
>>>>> behavior and performance of the server so drastically.
>>>>
>>>> It only acts on reads from the client. Normal non-POST requests arrive
>>>> in one or two packets, which would mean approx. 3 additional
>>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>>> imagine that this has a drastic impact on performance. And if it costs
>>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>>> type attacks which eat lots of memory.
>>>>
>>>> The general intention of the recent changes in default configs and
>>>> module selection/loading was to make it easier to only load those
>>>> modules that are really needed, have a reasonable default config, and
>>>> have the compiled-in default values be the same as those in the
>>>> example config files.
>>>
>>> Which means, build by default, disable by default. I think that keeps
>>> everyone happy. When abuse arrives, it's trivial to load.
>>
>> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
>> still in wide use, but mod_reqtimeout can provide a much more
>> reasonable out of the box configuration. I think we should keep it in
>> place by default.
>
> +1
>
What I am really opposed to is that the LoadModule causes such a
degradation in performance.
I am 100% +1 to adding conf commands to the default configuration in
the httpd.conf, but what I do not like is that having just a
LoadModule with nothing else causes reqtimeout to do work. It is too
trivial for people to have accidental load modules in many distros.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 15.11.2011 20:57, Jeff Trawick wrote:
> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>>
>>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>>
>>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>>
>>> wrote:
>>>>>
>>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>>
>>>>>> Author: pquerna
>>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>>> New Revision: 1202255
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>>> Log:
>>>>>> disable mod_reqtimeout if not configured
>>>>>
>>>>> Why that? We have just changed the default to be enabled in
>>>>> r1199447 and several developers at the hackathon agreed to this
>>>>> change.
>>>>
>>>> Didn't know it was discussed in depth at the hackathon, and there
>>>> wasn't any discussion on the list....
>>>>
>>>> It showed up quite quickly in my profiling of the Event MPM,
>>>> because every pull/push on the filters would cause a
>>>> apr_time_now() call.
>>>>
>>>> I don't really like that just by loading the module, it changes the
>>>> behavior and performance of the server so drastically.
>>>
>>> It only acts on reads from the client. Normal non-POST requests arrive
>>> in one or two packets, which would mean approx. 3 additional
>>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>>> imagine that this has a drastic impact on performance. And if it costs
>>> 1-2%, then that's a small cost compared to the impact of slowloris
>>> type attacks which eat lots of memory.
>>>
>>> The general intention of the recent changes in default configs and
>>> module selection/loading was to make it easier to only load those
>>> modules that are really needed, have a reasonable default config, and
>>> have the compiled-in default values be the same as those in the
>>> example config files.
>>
>> Which means, build by default, disable by default. I think that keeps
>> everyone happy. When abuse arrives, it's trivial to load.
>
> Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
> still in wide use, but mod_reqtimeout can provide a much more
> reasonable out of the box configuration. I think we should keep it in
> place by default.
+1
Rainer
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
>>
>> On Tuesday 15 November 2011, Paul Querna wrote:
>>>
>>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
>>
>> wrote:
>>>>
>>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>>>
>>>>> Author: pquerna
>>>>> Date: Tue Nov 15 15:49:19 2011
>>>>> New Revision: 1202255
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>>> Log:
>>>>> disable mod_reqtimeout if not configured
>>>>
>>>> Why that? We have just changed the default to be enabled in
>>>> r1199447 and several developers at the hackathon agreed to this
>>>> change.
>>>
>>> Didn't know it was discussed in depth at the hackathon, and there
>>> wasn't any discussion on the list....
>>>
>>> It showed up quite quickly in my profiling of the Event MPM,
>>> because every pull/push on the filters would cause a
>>> apr_time_now() call.
>>>
>>> I don't really like that just by loading the module, it changes the
>>> behavior and performance of the server so drastically.
>>
>> It only acts on reads from the client. Normal non-POST requests arrive
>> in one or two packets, which would mean approx. 3 additional
>> apr_time_now calls per request. I haven't done benchmarks, but I can't
>> imagine that this has a drastic impact on performance. And if it costs
>> 1-2%, then that's a small cost compared to the impact of slowloris
>> type attacks which eat lots of memory.
>>
>> The general intention of the recent changes in default configs and
>> module selection/loading was to make it easier to only load those
>> modules that are really needed, have a reasonable default config, and
>> have the compiled-in default values be the same as those in the
>> example config files.
>
> Which means, build by default, disable by default. I think that keeps
> everyone happy. When abuse arrives, it's trivial to load.
Timeout 60 isn't nearly as bad as the old Timeout 300 that is probably
still in wide use, but mod_reqtimeout can provide a much more
reasonable out of the box configuration. I think we should keep it in
place by default.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/15/2011 12:33 PM, Stefan Fritsch wrote:
> On Tuesday 15 November 2011, Paul Querna wrote:
>> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch<sf...@sfritsch.de>
> wrote:
>>> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>>>> Author: pquerna
>>>> Date: Tue Nov 15 15:49:19 2011
>>>> New Revision: 1202255
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>>>> Log:
>>>> disable mod_reqtimeout if not configured
>>>
>>> Why that? We have just changed the default to be enabled in
>>> r1199447 and several developers at the hackathon agreed to this
>>> change.
>>
>> Didn't know it was discussed in depth at the hackathon, and there
>> wasn't any discussion on the list....
>>
>> It showed up quite quickly in my profiling of the Event MPM,
>> because every pull/push on the filters would cause a
>> apr_time_now() call.
>>
>> I don't really like that just by loading the module, it changes the
>> behavior and performance of the server so drastically.
>
> It only acts on reads from the client. Normal non-POST requests arrive
> in one or two packets, which would mean approx. 3 additional
> apr_time_now calls per request. I haven't done benchmarks, but I can't
> imagine that this has a drastic impact on performance. And if it costs
> 1-2%, then that's a small cost compared to the impact of slowloris
> type attacks which eat lots of memory.
>
> The general intention of the recent changes in default configs and
> module selection/loading was to make it easier to only load those
> modules that are really needed, have a reasonable default config, and
> have the compiled-in default values be the same as those in the
> example config files.
Which means, build by default, disable by default. I think that keeps
everyone happy. When abuse arrives, it's trivial to load.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 15 November 2011, Paul Querna wrote:
> On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch <sf...@sfritsch.de>
wrote:
> > On Tue, 15 Nov 2011, pquerna@apache.org wrote:
> >> Author: pquerna
> >> Date: Tue Nov 15 15:49:19 2011
> >> New Revision: 1202255
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
> >> Log:
> >> disable mod_reqtimeout if not configured
> >
> > Why that? We have just changed the default to be enabled in
> > r1199447 and several developers at the hackathon agreed to this
> > change.
>
> Didn't know it was discussed in depth at the hackathon, and there
> wasn't any discussion on the list....
>
> It showed up quite quickly in my profiling of the Event MPM,
> because every pull/push on the filters would cause a
> apr_time_now() call.
>
> I don't really like that just by loading the module, it changes the
> behavior and performance of the server so drastically.
It only acts on reads from the client. Normal non-POST requests arrive
in one or two packets, which would mean approx. 3 additional
apr_time_now calls per request. I haven't done benchmarks, but I can't
imagine that this has a drastic impact on performance. And if it costs
1-2%, then that's a small cost compared to the impact of slowloris
type attacks which eat lots of memory.
The general intention of the recent changes in default configs and
module selection/loading was to make it easier to only load those
modules that are really needed, have a reasonable default config, and
have the compiled-in default values be the same as those in the
example config files.
Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Posted by Paul Querna <pa...@querna.org>.
On Tue, Nov 15, 2011 at 9:17 AM, Stefan Fritsch <sf...@sfritsch.de> wrote:
> On Tue, 15 Nov 2011, pquerna@apache.org wrote:
>
>> Author: pquerna
>> Date: Tue Nov 15 15:49:19 2011
>> New Revision: 1202255
>>
>> URL: http://svn.apache.org/viewvc?rev=1202255&view=rev
>> Log:
>> disable mod_reqtimeout if not configured
>
> Why that? We have just changed the default to be enabled in r1199447 and
> several developers at the hackathon agreed to this change.
>
Didn't know it was discussed in depth at the hackathon, and there
wasn't any discussion on the list....
It showed up quite quickly in my profiling of the Event MPM, because
every pull/push on the filters would cause a apr_time_now() call.
I don't really like that just by loading the module, it changes the
behavior and performance of the server so drastically.