httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dan Poirier <poir...@pobox.com>
Subject mod_reqtimeout backport (was: svn commit: r897527 - /httpd/httpd/branches/2.2.x/STATUS)
Date Tue, 12 Jan 2010 18:55:34 GMT
Some comments based on
<http://people.apache.org/~sf/mod_reqtimeout.2.2.patch>:

Code:

- The units are confusing in the computation and use of the rate_factor
  values.  rate_factor is computed as
 
    apr_time_from_sec(1)/min_rate

  where min_rate's units are bytes/second, so the units for the
  rate_factor are seconds^2/(10^6 * bytes).  Then in extend_timeout(),
  that gets multipled by a length in bytes, so we end up with units of
  seconds^2/10^6 where we really want microseconds.

  I think it would be clearer to write the computation of the
  rate_factor as

     rate_factor = APR_USEC_PER_SEC / min_rate

  so the units work out as usec/byte and the timeout ends up
  in usec.

  (Yes, you end up with the same numbers, but it took me quite a while
  to prove that to myself, given that the units weren't right.)

- I'd feel better if most of the AP_DEBUG_ASSERTs were replaced by code
  that would fail the request.  In production this could result in
  crashes at runtime if any of these bad return values actually happen.

- If headermax precedes headerinit on the RequestTimeout directive line,
  there's no check that headerinit <= headermax.  Similarly for bodymax
  and bodyinit.

Documentation:

- Should the module status still be "Experimental" if it's backported?

- Compatibility should show this available in 2.2.something if it's
  backported.

- s/timout/timeout/g

- Maybe an explanatory paragraph should be added, about xxxinit and
  xxxminrate and xxxmax and how they relate to each other and affect the
  timeout of requests.  That would resolve some questions I had as I was
  reading through this, like "why is this called headerinit instead of
  headerread?".


Dan

Mime
View raw message