Simplified quick_abort_pct code and improved its docs (#1921)

Instead of ignoring quick_abort_pct settings that would, together with
other conditions, abort a pending download of a 99-byte or smaller
response, Squid now honors quick_abort_pct for all response sizes. Most
Squids are not going to be affected by this change because default
quick_abort_min settings (16KB) prevent aborts of 99-byte responses even
before quick_abort_pct is checked.

Due to conversion from integer to floating point math, this change may
affect responses larger than 99 bytes as well, but these effects ought
to be limited to cases where the decision is based on a tiny difference
(e.g., receiving 1% more bytes would have triggered full download). In
most such cases, the decision could probably go either way due to
response header size fluctuations anyway.

Also updated quick_abort_pct documentation, primarily to clarify a
misleading statement: Squid did not and does not treat 16KB or smaller
responses specially in this context. The original statement was probably
based on quick_abort_min _default_ setting of 16KB, but statement
phrasing and placement hid that connection.

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 72f038730a6a019e9faa5831004387b4ae2a871f..d69ba5cb61ce778cf8c092c50a7226b8bd314a6f 100644 (file)
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -470,6 +470,7 @@ Thank you!
     Sergey Merzlikin <sm@smsoft.ru>
     Sergio Durigan Junior <sergiodj@sergiodj.net>
     Sergio Rabellino <rabellino@di.unito.it>
+    Shailesh Vashishth <shavashishth@gmail.com>
     Shigechika Aikawa <shige@luck.imasy.or.jp>
     Shmaya Frankel <shmaya.frankel@gmail.com>
     Silamael <Silamael@coronamundi.de>

diff --git a/doc/release-notes/release-7.sgml.in b/doc/release-notes/release-7.sgml.in
index f33c9992d670d82678e92f624baa6e0021f284bb..47039e782717c89dad32275966ec3e336377bfcf 100644 (file)
--- a/doc/release-notes/release-7.sgml.in
+++ b/doc/release-notes/release-7.sgml.in
@@ -194,6 +194,20 @@ This section gives an account of those changes in three categories:
        <tag>external_acl_type</tag>
        <p>Removed <em>%IDENT</em> format code with Ident protocol support.
 
+       <tag>quick_abort_pct</tag>
+       <p>Instead of ignoring <em>quick_abort_pct</em> settings that would,
+       together with other conditions, abort a pending download of a 99-byte or
+       smaller response, Squid now honors <em>quick_abort_pct</em> for all
+       response sizes. Most Squids are not going to be affected by this change
+       because default quick_abort_min settings (16KB) prevent aborts of 99-byte
+       responses even before <em>quick_abort_pct</em> is checked.
+       <p>Due to conversion from integer to floating point math, this change may
+       affect responses larger than 99 bytes as well, but these effects ought to
+       be limited to cases where the decision is based on a tiny difference (e.g.,
+       receiving 1% more bytes would have triggered full download). In most such
+       cases, the decision could probably go either way due to response header
+       size fluctuations anyway.
+
 </descrip>
 
 <sect1>Removed directives<label id="removeddirectives">

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 698c9294623ee6e24ea9932c7eaa356b415e59ea..d6e03f5fd3bcf7b3317119c1c3ad4c0aedcf3b7e 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -6636,16 +6636,14 @@ TYPE: int
 DEFAULT: 95
 LOC: Config.quickAbort.pct
 DOC_START
-       The cache by default continues downloading aborted requests
-       which are almost completed (less than 16 KB remaining). This
-       may be undesirable on slow (e.g. SLIP) links and/or very busy
-       caches.  Impatient users may tie up file descriptors and
-       bandwidth by repeatedly requesting and immediately aborting
-       downloads.
-
-       When the user aborts a request, Squid will check the
-       quick_abort values to the amount of data transferred until
-       then.
+       Continuing to download a cachable response after its request is aborted is
+       going to waste resources if the received response is not requested again.
+       On the other hand, aborting an in-progress download may effectively waste
+       (already spent) resources if the received cachable response is requested
+       again. Such waste is especially noticeable when, for example, impatient
+       users repeatedly request and then abort slow downloads. To balance these
+       trade-offs when a request is aborted during response download, Squid may
+       check quick_abort_* directives to decide whether to finish the retrieval:
 
        If the transfer has less than 'quick_abort_min' KB remaining,
        it will finish the retrieval.
@@ -6662,6 +6660,10 @@ DOC_START
 
        If you want retrievals to always continue if they are being
        cached set 'quick_abort_min' to '-1 KB'.
+
+       Many other conditions affect Squid decision to abort or continue download.
+       For example, Squid continues to download responses that feed other
+       requests but aborts responses with unknown body length.
 DOC_END
 
 NAME: read_ahead_gap

diff --git a/src/store_client.cc b/src/store_client.cc
index 6288b0dcbf8c10357b22a5884e2dc66544a56ac4..12b730fc4fcd688641bcd3333ceb85f3357b0054 100644 (file)
--- a/src/store_client.cc
+++ b/src/store_client.cc
@@ -984,13 +984,7 @@ CheckQuickAbortIsReasonable(StoreEntry * entry)
         return true;
     }
 
-    // XXX: This is absurd! TODO: For positives, "a/(b/c) > d" is "a*c > b*d".
-    if (expectlen < 100) {
-        debugs(90, 3, "quick-abort? NO avoid FPE");
-        return false;
-    }
-
-    if ((curlen / (expectlen / 100)) > (Config.quickAbort.pct)) {
+    if (curlen > expectlen*(Config.quickAbort.pct/100.0)) {
         debugs(90, 3, "quick-abort? NO past point of no return");
         return false;
     }