Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: domain of poirier@pobox.com designates
 208.72.237.35 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:subject
	:in-reply-to:references:date:message-id:mime-version
	:content-type; q=dns; s=sasl; b=FjTHZ8x1UoNTfgHziSPYWr2Fvnq6gSgE
	EY15XiQ7h3hciIbOBrYWW79YMtKbg37w2zcWJ13Doh3n2mLiQjjwfKNqFSJch40l
	DGnG9k0MwPt8xl7XYcms7U7f4kLUg4iUW7wlVnzK35WtODxQlPQkGHomNfgf27o2
	Di3enuc2cH0=
From: Dan Poirier <poirier@pobox.com>
To: dev@httpd.apache.org
Subject: mod_reqtimeout backport (was: svn commit: r897527 -
 /httpd/httpd/branches/2.2.x/STATUS)
In-Reply-To: <20100109195247.B53EC238899B@eris.apache.org> (sf@apache.org's
 message of "Sat, 09 Jan 2010 19:52:47 -0000")
References: <20100109195247.B53EC238899B@eris.apache.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (darwin)
Date: Tue, 12 Jan 2010 13:55:34 -0500
Message-ID: <yuapr5fgnkp.fsf@slappy.raleigh.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

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