From 79600062a8e97e095a96b6c1b7d8bc2b8e17dcce Mon Sep 17 00:00:00 2001
From: Shailesh Vashishth <shavashishth@gmail.com>
Date: Sat, 23 Nov 2024 13:41:03 +0000
Subject: [PATCH] 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.
---
 CONTRIBUTORS                        |  1 +
 doc/release-notes/release-7.sgml.in | 14 ++++++++++++++
 src/cf.data.pre                     | 22 ++++++++++++----------
 src/store_client.cc                 |  8 +-------
 4 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 72f038730a6..d69ba5cb61c 100644
--- 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 f33c9992d67..47039e78271 100644
--- 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 698c9294623..d6e03f5fd3b 100644
--- 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 6288b0dcbf8..12b730fc4fc 100644
--- 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;
     }