Re: [PATCH] Retry requests that failed due to a pconn race

On 17/02/2012 2:18 p.m., Alex Rousskov wrote:
> Hello,
>
> The attached patch against trunk restores Squid ability to retry
> connections that fail due to a pconn race condition. Currently, we serve
> an ERR_ZERO_SIZE_OBJECT "Bad Gateway" error in those cases.
>
> This patch is meant to reflect previous discussions on this topic but is
> simpler/tighter than my previous attempts that were posted as PREVIEW
> patches.
>
>
> Thank you,
>
> Alex.
>

I'm not sure about:

+ // XXX: stale serverConn may remain set even though we are opening new conn

** This should not matter, the next stage will either retry failed connect using it yet again, or reset serverConn to the newely opened Comm::Connection (itself). If you want to be certain about that you can move the "serverConn = temp;" line up out of the if() condition.

We are more likely to have problems from the local port number of the serverConn being non-0 on the retries. Since the IP is set Squid will use bind() to ensure the outgoing IP is retained and bind() does not like ports which are in TIME_WAIT from previous use. Sorry I missed this on the last audit.
The place to add serverConn->local.SetPort(0) is retryOrBail() alongside the new debugs()

Otherwise it seems good. +1 with those two tweaks.

Amos
Received on Fri Feb 17 2012 - 08:59:17 MST