ICAP Persistent Connections vs Retries (with code review)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

ICAP Persistent Connections vs Retries (with code review)

Juan Ramírez
Hi,

Mi name is Juan, I am a Software Engineer from Uruguay. I think this message is more suited to the squid-dev mailing list due to the developer-oriented nature of the message but, given that the development list is for people who actually contributes code to Squid, I chose to post here.

I started using Squid a few days ago in order to test its content-adaptation capabilities. The plan was to test the ICAP implementation first and then maybe try the eCAP API as well.

In order to test ICAP, I based my code in the open source PYICAP project, I also ran some tests using the C-ICAP server.

It came to my attention that, even when persistent connections is enabled, Squid closes the ICAP connection everytime a new request arrives, like this:

1. A new request arrives
2. Squid creates a connection to the ICAP server
3. Content is adapted and returned to the client
4. Squid returns the connection to the connection pool
5. A new requests arrives
6. Squid closes the active connection
7. Squid opens a new connection to the ICAP server

Note: I am using only the RESPMOD method.

This tests was performed using Squid 3.5.x. I downloaded the last available tarball (squid-4.0.19), and run the same test, which gave the same results.

So I started digging into the source code and found something interesting:

File:
    /src/adaptation/icap/ServiceRep.cc
Function: 
    Adaptation::Icap::ServiceRep::getConnection(bool retriableXact, bool &reused)

Code:
    if (retriableXact)
        connection = theIdleConns->pop();
    else {
        theIdleConns->closeN(1);
    }

The connection is taken from the set of idle connections  ONLY if retriableXact is set to true.

File:
    /src/adaptation/icap/Xaction.cc
Function:
    Adaptation::Icap::Xaction::openConnection()

Xaction uses the member variable 'isRetriable' as parameter for ServiceRep::getConnection.

The function Xaction::disableRetries() is called from a lot of places, but in my test case was called only two times (per request):

   1. In the function Adaptation::Icap::Xaction::noteCommRead, when Comm::ReadNow returns Comm::OK.

   2. In the function Adaptation::Icap::Xaction::closeConnection, because reuseConnection is set to true.

I am not sure if Xaction objects are reusable, it seems that they are because setting isRetriable to false is affecting the way connections are reused.

Anyway, I think the concept of retriability shuld not be confused with the concept of reusability, but I haven't gone deep enough in order to ensure that.

I appreciate your comments on this. Don't hesitate to contact me should any additional information be needed.


--
Juan
:wq


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: ICAP Persistent Connections vs Retries (with code review)

Amos Jeffries
Administrator
On 12/05/17 12:49, Juan Ramírez wrote:
> Hi,
>
> Mi name is Juan, I am a Software Engineer from Uruguay. I think this
> message is more suited to the squid-dev mailing list due to the
> developer-oriented nature of the message but, given that the
> development list is for people who actually contributes code to Squid,
> I chose to post here.

Membership is for those people, the list itself acts as a focal point
for anyone such as yourself with this query to contact them without
having to know particular peoples contacts.


As to your issue;

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.

The behaviour which you describe was designed to prevent the ICAP pool
from absorbing all available TCP resources through these constant brand
new TCP connections being needed then added to the pool. Which would
leave the proxy unable to receive HTTP traffic as there would be no TCP
sockets/ports available for non-ICAP connections.

If your service only (or mostly) has non-retriable transactions occuring
it can be more efficient to disable persistence o speed up the turn
around on TCP socket closures. Persistence is useful for when you have
many retriable transactions occuring which can use pooled connections
and be re-tried if something barfs at the TCP level.


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).

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. That is the noteCommRead call - to prevent the connection
being pooled if the connection suddenly closes before it completes. This
should not have any effect on whether a pooled connection was used to
start with, but will affect whether a pooled connection gets used on the
retry attempt to prevent the same issue/delay repeating indefinitely to
the same client.

(PS. Alex knows a lot more about the details of this design than me, so
if he says anything contradicting the above go with his).

Amos

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: ICAP Persistent Connections vs Retries (with code review)

Alex Rousskov
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.

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: 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 <[hidden email]> 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


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: ICAP Persistent Connections vs Retries (with code review)

Alex Rousskov
On 05/12/2017 01:17 PM, Juan Ramírez wrote:

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

You are implying that idle persistent connections are used for retries.
They are not (or, at least, should not be). Idle persistent connections
are used for new requests that can be retried.

I am deleting most of my carefully written answers and skipping most of
your email until the place where you discovered problems without your
earlier statements/questions :-(.


> * Why does the size of the payload affect the way connections are reused

Because to resend the request, Squid has to have the payload to send.
Thus, to resend a 10GB request, Squid has to buffer a 10GB request.
Squid does not want to do that for, hopefully, obvious reasons.


> * Does Squid need a finer control on this matter in order to ensure
> connection reusability is not affected by the size of the payload?

No. Connection reusability is affected by the size of the payload. It is
the unavoidable consequence of combining finite memory with potentially
infinite response sizes.


> I understand much more now than when I started writing this message.

FWIW, you would save some of us a lot of time if you do not send
portions that you later (but in the same email!) discovered to be
incorrect or irrelevant. This mailing list is not a blog; somebody may
actually be reading and responding to what you write.


> + I am not sure if it needs finer control, but the fact that is closing
> connections in advance seems dangerous.

There is no danger in a client closing an idle connection.


> + It also seems dangerous to me this protection mechanism given the fact
> that opening/closing TCP connections everytime can become a bottleneck.

There is some performance overhead, but not danger. High quality
optimizations are welcomed.


> I disabled the call to `theIdleConns->closeN(1);`
> Do you think there would be any issues with this change?

Yes, I do:

* In some environments, your patched Squid is likely to open too many
connections, violating various OS limits and/or admin expectations.

* In some environments, your patched Squid is likely to stop servicing
requests after hitting one of the open connection limits, unable to open
a new connection for an (unretriable) transaction, with a connection
pool full of idle connections.

The code you do not like was added because Squid was experiencing such
problems in some real-world environments (both HTTP and ICAP). It _can_
be improved, and quality patches are welcomed, but it should not be
disabled (in the official version).


HTH,

Alex.

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: ICAP Persistent Connections vs Retries (with code review)

Eliezer Croitoru
In reply to this post by Juan Ramírez
Hey Juan,

Without delving into ICAP and HTTP semantics you should notice\remember one thing:
Proxies which are performing as an ICAP client should not care about a scenario which a single TCP connection is being used for a single query.
If indeed you find that this scenario, which many connections are being opened and closed towards the ICAP service, affects your service I believe you should rethink things over.
The directions you should think are:
 - Software(OS or Squid or ICAP service)
 - Hardware (CPU, RAM)
 - Network (Network interfaces, Cables Switches)

I can testify that for HTTP content filtering and adaptation there are much more efficient pieces of software out there(which includes internal content adaptation).

With the above in mind you should not really care too much about new TCP connections per request.
If you see some performance degradation in your environment I would recommend your to create a "check list" which will help you to decide what direction you should choose.

All The Bests,
Eliezer

* Let me know if you need some help with the check list or software which you can test.

----
http://ngtech.co.il/lmgtfy/
Linux System Administrator
Mobile: +972-5-28704261
Email: [hidden email]


From: squid-users [mailto:[hidden email]] On Behalf Of Juan Ram?rez
Sent: Friday, May 12, 2017 3:50 AM
To: [hidden email]
Subject: [squid-users] ICAP Persistent Connections vs Retries (with code review)

Hi,

Mi name is Juan, I am a Software Engineer from Uruguay. I think this message is more suited to the squid-dev mailing list due to the developer-oriented nature of the message but, given that the development list is for people who actually contributes code to Squid, I chose to post here.

I started using Squid a few days ago in order to test its content-adaptation capabilities. The plan was to test the ICAP implementation first and then maybe try the eCAP API as well.

In order to test ICAP, I based my code in the open source PYICAP project, I also ran some tests using the C-ICAP server.

It came to my attention that, even when persistent connections is enabled, Squid closes the ICAP connection everytime a new request arrives, like this:

1. A new request arrives
2. Squid creates a connection to the ICAP server
3. Content is adapted and returned to the client
4. Squid returns the connection to the connection pool
5. A new requests arrives
6. Squid closes the active connection
7. Squid opens a new connection to the ICAP server

Note: I am using only the RESPMOD method.
<<SNIP>>

--
Juan
:wq


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users