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 Thu, 17 Nov 2011 12:32:39 GMT
On Wed, Nov 16, 2011 at 2:26 PM, Jeff Trawick <trawick@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 2:23 PM, Jeff Trawick <trawick@gmail.com> wrote:
>> 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.
>
> 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.

Mime
View raw message