Re: [PATCH] cache_peer standby=N

On 04/29/2014 05:48 AM, Amos Jeffries wrote:
> On 29/04/2014 8:46 a.m., Alex Rousskov wrote:
>> On 04/27/2014 10:02 PM, Amos Jeffries wrote:
>>
>>> We should state the problem with idles clearly (yes it is difficult to
>>> word),
>>
>> We already do that:
>>
>>> + max-conn limit works poorly when there is a relatively
>>> + large number of idle persistent connections with the
>>> + peer because the limiting code does not know that
>>> + Squid can often reuse some of those idle connections.

> What about:
> "
> max-conn works poorly with persistent connections and may prevent a peer
> being selected when there are idle connections because the limiting code
> does not know whether Squid can reuse some of those idle connections.
> "

Sure, I would just emphasize that a peer may be excluded only when it
reached the limit, not just when it has some pconns:

"
max-conn currently works poorly with idle persistent connections: When a
peer reaches its max-conn limit, and there are idle persistent
connections to the peer, the peer may not be selected because the
limiting code does not know whether Squid can reuse those idle connections.
"

>>> or we fix that problem (see below) and update the documentation
>>
>> The change is not trivial, so I do not think we should be forced to do
>> that as a part of this project. There are many problems with idle
>> connections, and we are not making them worse by adding the standby
>> pools, quite the opposite. It feels like we are being penalized for
>> improving documentation of ancient problems.

> If you want to do it as a followup fine. I just do not see a particular
> need to delay fixing a bug with a (now) known solution.

The solution you have outlined is incomplete. The correct solution will
involve more work than you think. There are worse problems to work on.
This problem is old, and the new feature being reviewed does not make it
worse. IMO, this situation clearly falls into the "quality patches
welcomed" category, not the "if you want standby feature to be accepted,
you must [promise to] fix the idle connection problem" category.

Alex.
Received on Tue Apr 29 2014 - 14:11:12 MDT