]> git.ipfire.org Git - thirdparty/squid.git/commit - src/http.cc
Fix two read-ahead problems related to delay pools (or lack of thereof).
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Fri, 3 Mar 2017 22:15:10 +0000 (15:15 -0700)
committerAlex Rousskov <rousskov@measurement-factory.com>
Fri, 3 Mar 2017 22:15:10 +0000 (15:15 -0700)
commitf1ba1fba3f8da3a06496f200862b9201876ec013
treef7ec7f4450c874322af8f5db8957fa65cee20b85
parent2761b12a0f0180df00820619a53d02c847e615d1
Fix two read-ahead problems related to delay pools (or lack of thereof).

1. Honor EOF on Squid-to-server connections with full read ahead buffers
   and no clients when --enable-delay-pools is used without any delay
   pools configured in squid.conf.

Since trunk r6150.

Squid delays reading from the server after buffering read_ahead_gap
bytes that are not yet sent to the client. A delayed read is normally
resumed after Squid sends more buffered bytes to the client. See
readAheadPolicyCanRead() and kickReads().

However, Squid was not resuming the delayed read after all Store clients
were gone. If quick_abort prevents Squid from immediately closing the
corresponding Squid-to-server connection, then the connection gets stuck
until read_timeout (15m), even if the server closes much sooner, --
without reading from the server, Squid cannot detect the connection
closure. The affected connections enter the CLOSE_WAIT state.

Kicking delayed read when the last client leaves fixes the problem. The
removal of any client, including the last one, may change
readAheadPolicyCanRead() answer and, hence, deserves a kickReads() call.

Why "without any delay pools configured in squid.conf"? When classic
(i.e., delay_pool_*) delay pools are configured, Squid kicks all delayed
reads every second. That periodic kicking is an old design bug, but it
resumes stuck reads when all Store clients are gone. Without classic
delay pools, there is no periodic kicking. This fix does not address
that old bug but removes Squid hidden dependence on its side effect.

Note that the Squid-to-server connections with full read-ahead buffers
still remain "stuck" if there are non-reading clients. There is nothing
Squid can do about them because we cannot reliably detect EOF without
reading at least one byte and such reading is not allowed by the read
ahead gap. In other words, non-reading clients still stall server
connections.

While fixing this, I moved all CheckQuickAbort() tests into
CheckQuickAbortIsReasonable() because we need a boolean function to
avoid kicking aborted entries and because the old separation was rather
awkward -- CheckQuickAbort() contained "reasonable" tests that were not
in CheckQuickAbortIsReasonable(). All the aborting tests and their order
were preserved during this move. The moved tests gained debugging.

According to the existing test order in CheckQuickAbortIsReasonable(),
the above problem can be caused by:

* non-private responses with a known content length
* non-private responses with unknown content length, having quick_abort_min
  set to -1 KB.

2. Honor read_ahead_gap with --disable-delay-pools.

Since trunk r13954.

This fix also addresses "Perhaps these two calls should both live
in MemObject" comment and eliminates existing code duplication.
src/MemObject.cc
src/http.cc
src/store.cc
src/store_client.cc