Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified quick_abort_pct code and improved its docs #1921

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ Thank you!
Sergey Merzlikin <[email protected]>
Sergio Durigan Junior <[email protected]>
Sergio Rabellino <[email protected]>
Shailesh Vashishth <[email protected]>
Shigechika Aikawa <[email protected]>
Shmaya Frankel <[email protected]>
Silamael <[email protected]>
Expand Down
14 changes: 14 additions & 0 deletions doc/release-notes/release-7.sgml.in
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
22 changes: 12 additions & 10 deletions src/cf.data.pre
Original file line number Diff line number Diff line change
Expand Up @@ -6626,16 +6626,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.
Expand All @@ -6652,6 +6650,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
Expand Down
8 changes: 1 addition & 7 deletions src/store_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
vshailesh marked this conversation as resolved.
Show resolved Hide resolved
debugs(90, 3, "quick-abort? NO past point of no return");
return false;
}
Expand Down