On Wed, Nov 16, 2011 at 2:26 PM, Jeff Trawick <[email protected]> wrote: > On Wed, Nov 16, 2011 at 2:23 PM, Jeff Trawick <[email protected]> wrote: >> On Wed, Nov 16, 2011 at 11:51 AM, Jeff Trawick <[email protected]> wrote: >>> On Wed, Nov 16, 2011 at 11:20 AM, Paul Querna <[email protected]> wrote: >>>> On Wed, Nov 16, 2011 at 2:44 AM, Rainer Jung <[email protected]> >>>> wrote: >>>>> On 15.11.2011 20:57, Jeff Trawick wrote: >>>>>> >>>>>> On Tue, Nov 15, 2011 at 2:32 PM, William A. Rowe Jr. >>>>>> <[email protected]> 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<[email protected]> >>>>>>>> >>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> On Tue, 15 Nov 2011, [email protected] 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.
