[squid-users] server persistent connections and cache

[Amos Jeffries]

On 27/07/18 16:18, Alex Rousskov wrote:
> On 07/26/2018 09:15 PM, Amos Jeffries wrote:
>> On 27/07/18 13:31, Alex Rousskov wrote:
>>> On 07/26/2018 05:47 PM, Vishali Somaskanthan wrote:
>>>
>>>> By re-use I meant to say that the server-connection S (TCP + SSL) is
>>>> re-used across 2 client connections (C1 and C2), from the same client
>>>> one after the other is torn down. I, presume that
>>>> “/server_persistent_connection on/” allows for such a use-case.
>>>
>>> server_persistent_connection controls whether a single Squid-to-server
>>> connection can carry more than one request. It does not (or should not)
>>> control re-pinning.
> 
>> Also, pinned connections should never be added to the persistent pool
>> after pinning. Doing so is a bug IMO.
> 
> Agreed.
> 
> 
>> Sending the client handshake to a server makes the TLS end-to-end
>> stateful between the client<->server. So *that* is the first point at
>> which pinning is required by Squid.
> 
> At TLS level, yes. However, one could argue that Squid should honor a
> (higher level) client and server assumption that they are talking to
> each other (at HTTP+ level).

One could, but that is dependent on what the admin policy is.
Configuring persistence on/off is the way to control that, not pinning.

> We will probably break fewer transactions
> that way. With that idea in mind, the "first point" becomes establishing
> a TCP tunnel with the server, even if no client Hello pieces are forwarded.
> 

HTTP(S) being stateless, at that level any endpoint relying on implicit
state is non-compliant and buggy. HTTP(S) agents are explicitly required
to include any necessary state references in each message generated.

It is *nice* not to result in visible errors, but not a requirement we
should stick to at cost of proper behaviour. HTTP and extensions also
specify required error handling in most cases where it matters.


> 
>> Bumping at step1 does not involve any server information. So no pinning
>> required. The client is talking to *Squid* over TLS independent of how
>> the response is fetched.
> 
> True for TLS level. Not necessarily true for higher levels as discussed
> above.
> 

The client accepted proof that Squid *was* that origin (false or not)
order to reach said higher levels. Pinning at a later time due to higher
level stateful situation is not relevant to the bumping code actions and
not something we need to consider at the present C2 use of S after C1
should have pinned it.

The only cross-level requirement is to provide the surety that pinning
is a one-way action. Any level setting it makes the connections fate shared.


> 
>>>> Case 1: We see that no pinning happens i.e. pinConnection() is not
>>>> called at all. C1->S gets established, C1 is closed and then C2 re-uses S
> 
>>> The code just happens to work this way (evidently). It is not something
>>> I would rely on until the matter is discussed and settled.
> 
>> I think the described behaviour of C2 using S after C1 has pinned the
>> connection is a bug.
> 
> Your earlier "Sending the client handshake to a server" and "Bumping at
> step1" statements contradict this statement AFAICT because both C1 and
> C2 were bumped at step1 in "Case 1".
> 

 "after C1 has pinned" - if no pinning happens at all the whole
statement is irrelevant. Sorry for confusion.


> FWIW, I would default to honor the tunnel until somebody presents a
> convincing argument for making the behavior configurable. That is, like
> you said, treat C2 reusing S (or, more precisely, not closing S when C1
> is closed) as a Squid bug.
> 

What I'm most confused about here is why S is closed when C1 dies with
non-pinning.  _unless_ there is pinning between them they should not be
that closely fate sharing.

Is the HTTPS message on both C1 and S saying "Connection: close" perhapse?
 If so the closure relationship is an illusion.
 But then, why would C2 be using a connection currently in-use by C1 in
a way that starts *after* C1 closure? (so not collapsed forwarding AFAIK)

Something smells fishy.


> 
>> * S should not be pooled as persistent until the client which triggered
>> its TCP open has finished with it (eg after the TLS handshake completes).
> 
>> * Once the handshake begins sending client data, whichever client was
>> opening it should pin it.
> 
> I agree that pinned connections should not be pooled. Bugs
> notwithstanding, they are not pooled today IIRC.
> 
> I am not sure you meant that, but I doubt a hypothetical feature of
> pooling before-sending-handshake/before-pinning connections is a good
> idea, but it does not contradict "pinned connections are not pooled"
> principle. In practice, once the TCP connection is established, the
> requester pins it immediately (if needed) so there is no opportunity or
> need for pooling.
> 

Off-topic; TLS has connections that do not use handshakes at all, which
are becoming more common in TLS/1.3. So I believe such a feature may be
coming one day, but irrelevant right now.


> 
>> * The sequence of the above two should result in S being pinned before
>> it is ever considered for the persistence pool. Which should cause it to
>> be excluded from the servers pool.
> 
> Agreed.
> 
> 
>> So IMO when these things are working properly, C2 server selection
>> should never even see that S exists let alone be able to use it.
> 
> The open question is whether S should be pinned in the case where C1 is
> bumped at step1 (i.e., "Case 1" in Vishali's email). If S should be
> pinned, then I agree with the above statement. If S should not be
> pinned, then S is just a regular Squid-to-server connection that can be
> pooled.
> 

IMO in *that* specific case S should not be pinned. Because pinning
would prevent the very reasonable actions of handling server failures by
opening new Sn connections to finish incomplete responses to the client,
or retaining a server connection for the quick_abort features.

Squid should instead be ensuring that the server handshake at TLS level
matches what it would have used for C2 clean handshake. I may have
missed it, but that is not yet being done (just TCP equivalence, not TLS).

Amos