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:26:30 GMT
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 ;)

Mime
View raw message