[squid-users] ICAP Persistent Connections vs Retries (with code review)

[Juan Ramírez]

Hi,

Thank you all for such detailed responses.

I still don't understand whether it is possible to reuse ICAP connections
for cases other than retries.

As far as I know, Squid is able to save connections in a pool called
`theIdleConns`.
Can these connection be reused for other transactions in the future?

The flow I am trying to accomplish is the following:

1. Client performs a requests and a response arrives
2. Squid catches the response and invokes the ICAP adaptation routine
3. A new connection the the ICAP server is created
4. The RESPMOD transaction is issued successfully
5. The connection is returned to the pool
6. The response is then sent to the client

... Moments later:

7. Another client performs a request a a response arrives
8. Squid catches the response and invokes the ICAP adaptation routine
9. Squid reuses the connection which was created previously
10. The RESPMOD transaction is issued successfully
11. The connection is returned to the pool
12. The response is then sent to the client

That behavior does not happen, going back to this code:

    if (retriableXact)
        connection = theIdleConns->pop();
    else
        theIdleConns->closeN(1);

The line `theIdleConns->closeN(1);` is the one that gets executed all the
time with no exception. Well, it is actually an exception to the case, and
I'll explain it later. Have in mind that I modified the code in order to
prove that:

    if (retriableXact) {
        debugs(93, 1, HERE << "Calling theIdleConns->pop()");
        connection = theIdleConns->pop();
    } else {
        debugs(93, 1, HERE << "Calling theIdleConns->closeN(1)");
        theIdleConns->closeN(1);
    }


Based on that log, I could confirm that connections are reused properly
only for small response sizes.
Digging a little bit into the source code I finally understood what Alex
mean with "Squid can buffer the entire HTTP message body.":
  Retries are disabled if the message body is to big, this is a protective
measure which prevents from using to much RAM, it is clearly documented and
I understand it.

For those curious enough, the file in question is `ModXact.cc` (functions `
decideOnRetries()` and `canBackupEverything()`).

Wrapping up:

* Why does disabling retries in a connection dooms the connection?
* Why does the size of the payload affect the way connections are reused
* Does Squid need a finer control on this matter in order to ensure
connection reusability is not affected by the size of the payload?

In the first response, Amos wrote the following:

>> Requests which are not retriable are not able to be re-sent if it turns
out
>> they got even partially delivered on a pre-opened persistent connection
>> which happened to be in the process of closing by the other endpoint
>> unknown to Squid. For example if the timing of the write to deliver the
>> ICAP RESPMOD headers overlapped with the TCP FIN/RST packets
>> coming from a router along the network path, or the ICAP service itself.

>> As such those requests need a new TCP connection to be opened to
>> guarantee the absence of immediate closure. When they complete with
>> their transaction it gets added to the pool for other traffic to use if
they can.


This allows me to possibly answer the questions above and create new ones
(a clever reader may notice I understand much more now than when I started
writing this message).

+ Squid uses a self-defense mechanisms which allows to do two things:
  1. Use a fresh connection for non-retriable transactions, minimizing the
possibility of using a connection which may be in the process of being
closed.
  2. Control the number of open connections so it doesn't go out of bounds.
This is a little redundant because `ServiceRep::putConnection()` is already
taken care of that.
+ I am working with payloads that have more than 64K in size, which makes
me fall in the conditions stated above.
+ I am not sure if it needs finer control, but the fact that is closing
connections in advance seems dangerous.
+ It also seems dangerous to me this protection mechanism given the fact
that opening/closing TCP connections everytime can become a bottleneck.

So, and this is the final question, I promise:

I disabled the call to `theIdleConns->closeN(1);` and it seems to work
well.
The pool doesn't go out of bounds because the connection is closed inside
`ServiceRep::putConnection()` whenever `ServiceRep::excessConnections()`
returns true.

Do you think there would be any issues with this change?


Thanks in advance

Juan



On Fri, May 12, 2017 at 11:08 AM, Alex Rousskov <
rousskov at measurement-factory.com> wrote:

> On 05/11/2017 07:30 PM, Amos Jeffries wrote:
>
> > Requests which are not retriable are not able to be re-sent [...]
> > As such those requests need a new TCP connection to be opened to
> > guarantee the absence of immediate closure. When they complete with
> > their transaction it gets added to the pool for other traffic to use if
> > they can.
>
> The above is accurate, including the snipped parts documenting the
> rationale. BTW, since HTTP and ICAP share the same basic connection
> persistency model, you may find more information reading about these
> general problems in HTTP-related documents.
>
>
> > As far as I know RESPMOD transactions should all be retriable unless the
> > body payload length is unknown at the HTTP level (lack of Content-Length
> > header).
>
> This is inaccurate. A REQMOD or RESPMOD transaction is deemed retriable
> at the time when a connection is chosen/open if:
>
> * Squid can send a preview or
> * Squid can buffer the entire HTTP message body.
>
> The "known length" aspect affects the second bullet, but that bullet
> covers more cases than just messages with Content-Length. And there may
> be more conditions in the code than the bullets cover -- I have not
> checked.
>
>
> > They can also become non-retriable when the response from the ICAP
> > server has been received by Squid and already started delivery to the
> > HTTP client.
>
> This is accurate (including the snipped part about the irrelevance to
> the connection choice).
>
>
> HTH,
>
> Alex.
>
>


-- 
Juan
:wq
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-users/attachments/20170512/56b48745/attachment.html>