httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1202255 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Date Wed, 16 Nov 2011 19:23:08 GMT
On Wed, Nov 16, 2011 at 11:51 AM, Jeff Trawick <trawick@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <paul@querna.org> wrote:
>> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <rainer.jung@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.
>>>> <wrowe@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.

Mime
View raw message