On 09/01/2012 03:26 AM, Amos Jeffries wrote: > On 1/09/2012 5:05 a.m., Alex Rousskov wrote: >> * For the conn-open-timeout patch: >> >> Most importantly, the timeout handler should abort the ConnOpener job on >> the spot rather than go through one more select() try.
> ConnOpener should be obeying squid.conf connect_retries option for > number of times it closes the socket, re-open and re-connect()s. Yes, but IMO the timeout has higher priority than connect_retries. In other words, if we timed out, we should bail, even if we tried fewer than connect_retries times. Squid.conf documentation seems to agree with me: > This sets the maximum number of connection attempts made for each > TCP connection. The connect_retries attempts must all still > complete within the connection timeout period. Three designs are possible: 1. Bail after N retries, using T second timeout for each try. 2. Bail after N retries or T seconds, whichever comes first. 3. Bail after N retries (using T1 second timeout for each try) or after T2 seconds, whichever comes first. We should not do #1 IMO because we want to limit the total transaction wait time in most cases (and care less about lower-level details such as the number of retries). Documentation says that we do #2. I think we should do that. If we do not do that, we should fix the code. Specifically, ConnOpener's timeout handler should not call the connect() method. #3 is the most flexible approach but it requires more squid.conf options to distinguish a single-connect timeout from the aggregate connect timeout. I am not against that, but #2 seems sufficient until we have well-justified requests for #3. Hope this clarifies, Alex.
