]> git.ipfire.org Git - thirdparty/squid.git/commit
Fixed forward_max_tries documentation and implementation (#277)
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Wed, 28 Nov 2018 23:55:29 +0000 (23:55 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Tue, 4 Dec 2018 15:42:22 +0000 (15:42 +0000)
commit3eebd267df415536641878d692117caca30ca069
treee1f9e1c606dae1688c24a17e821eb9c55de0af26
parent14bf2426f73c3c72fad12b59886999c04dab4d8b
Fixed forward_max_tries documentation and implementation (#277)

Before 1c8f25b, FwdState::n_tries counted the total number of forwarding
attempts, including pinned and persistent connection retries. Since that
revision, it started counting just those retries. What should n_tries
count? The counter is used to honor the forward_max_tries directive, but
that directive was documented to limit the number of _different_ paths
to try. Neither 1c8f25b~1 nor 1c8f25b code matched that documentation!

Continuing to count just pinned and persistent connection retries (as in
1c8f25b) would violate any reasonable forward_max_tries intent and admin
expectations. There are two ways to fix this problem, synchronizing code
and documentation:

* Count just the attempts to use a different forwarding path, matching
  forward_max_tries documentation but not what Squid has ever done. This
  approach makes it difficult for an admin to limit the total number of
  forwarding attempts in environments where, say, the second attempt is
  unlikely to succeed and will just incur wasteful delays (Squid bug
  4788 report is probably about one of such use cases). Also,
  implementing this approach may be more difficult because it requires
  adding a new counter for retries and, for some interpretations of
  "different", even a container of previously visited paths.

* Count all forwarding attempts (as before 1c8f25b) and adjust
  forward_max_tries documentation to match this historical behavior.
  This approach does not have known unique flaws.

Also fixed FwdState::n_tries off-by-one comparison bug discussed during
Squid bug 4788 triage.

Also fixed admin concern behind Squid bug 4788 "forward_max_tries 1 does
not prevent some retries": While the old forward_max_tries documentation
actually excluded pconn retries, technically invalidating the bug
report, the admin now has a knob to limit those retries.
src/FwdState.cc
src/FwdState.h
src/cf.data.pre