Remove doomed attempts to reforward pinned requests

[rousskov]

AsyncJob.cc(100) mustStop: Ftp::Relay will stop, reason: exception
...
FATAL: check failed: !request->pinnedConnection()
    exception location: FwdState.cc(1119) connectStart

Since February 2019 commit 3dde9e5, retries of "pinned" requests¹
using a new Squid-to-server connections are meant for FTP and
connection-based authentication traffic only. Back then, we agreed that
such re-pinning is wrong², and different mechanisms should be used to
handle forwarding errors in those cases, but we preserved the
corresponding functionality in fear of breaking "working" cases³. On
that preserved code path, request->pinnedConnection() is true at
pinnedCanRetry() check time.

Since September 2019 commit daf8070, connectStart() throws if
request->pinnedConnection() is true, effectively breaking retries for
nearly all preserved cases mentioned above! The only pinned retries that
could hypothetically continue working after that commit are those where
ConnStateData job had been destroyed between pinnedCanRetry() and
connectStart(), and those rare cases are not interesting since
ConnStateData destruction ought to abort request forwarding anyway.

Both 2019 commits are present in v5.0.1 and beyond. We are not aware of
any associated problems. Thus, our fears were excessive, and we can
remove code that preserves problematic functionality. If we discover a
need to retry pinned FTP or connection-authenticated requests via a new
connection, we will add mechanisms to support that functionality.

Removing this code also helps avoid FATAL errors observed during recent
FTP work.

CONNECT tunnels

Existing tunnel.cc code ignores request->flags.pinned when deciding
whether to retry a failed tunneling attempt. This change does not affect
the corresponding "check pinned connections" TODO in that tunneling
code. FWIW, that TODO is limited to tunnelStart(); switchToTunnel()
cases already do not retry due to a "committed to server" retry ban on
their code path. It is likely that pinned retries should be disabled in
tunneled cases as well, but that adjustment deserves a dedicated
analysis/change.

Footnotes

1. Here, a "pinned" request is, roughly speaking, a request sent by
   Squid over a Squid-to-server connection that was kept/provided by a
   client-to-Squid connection manager (ConnStateData). See usePinned(). ↩

2. "automatically re-opening a PINNED connection from the middleware
   is invalid in todays networks" at
   https://github.com/squid-cache/squid/pull/351#discussion_r250015937 ↩

3. "keep re-pinning as is" at
   https://github.com/squid-cache/squid/pull/351#discussion_r253757986 ↩

[rousskov]

This PR stems from recent commit 51d1ddb work, but it is not required for that commit changes to function properly: That commit does not cause the FATAL error quoted in this PR description. That error was observed in an earlier variation of that code.

Said that, I suspect that similar errors will sooner or later resurface as more code throws on errors. And even if that never happens, it is a good idea to remove this not-working and complex/dangerous code anyway.

Disclaimer: I tested FTP but not connection-based authentication cases. Removed code does not distinguishes the two categories though -- it ought to hit the same !request->pinnedConnection() assertion in FwdState::connectStart() AFAICT.

P.S. We are working on one more 51d1ddb followup PR.