Re: Client-side pconns and r10728

I don't get how any of these changes can be relevant to the bug in
question.

Regards
Henrik

fre 2010-08-13 klockan 19:42 -0600 skrev Alex Rousskov:
> On 08/12/2010 03:37 AM, Amos Jeffries wrote:
> > ------------------------------------------------------------
> > revno: 10728
> > committer: Amos Jeffries<squid3_at_treenet.co.nz>
> > branch nick: trunk
> > timestamp: Thu 2010-08-12 21:37:14 +1200
> > message:
> > Author: Stephen Thorne<stephen_at_thorne.id.au>
> > Bug 2936: NTLM-Authenticate 407 and Proxy-Connection: Close in same response.
> >
> > Squid default from the days of HTTP/1.0 was to close connections unless
> > keep-alive was explicitly known. This changes the default to send
> > keep-alive unless we have a good reason to close.
> > modified:
> > src/client_side_reply.cc
>
> > === modified file 'src/client_side_reply.cc'
> > --- a/src/client_side_reply.cc 2010-07-13 14:27:25 +0000
> > +++ b/src/client_side_reply.cc 2010-08-12 09:37:14 +0000
> > @@ -1383,6 +1383,9 @@
> > } else if (fdUsageHigh()&& !request->flags.must_keepalive) {
> > debugs(88, 3, "clientBuildReplyHeader: Not many unused FDs, can't keep-alive");
> > request->flags.proxy_keepalive = 0;
> > + } else if (request->http_ver.major == 1 && request->http_ver.minor == 1) {
> > + debugs(88, 3, "clientBuildReplyHeader: Client is HTTP/1.1, send keep-alive, no overriding reasons not to");
> > + request->flags.proxy_keepalive = 1;
> > }
> >
>
> Persistent connections have been semi-broken since 3.0, but was the
> above fix discussed somewhere? I think it contradicts the overall flow
> of the persistency handling code in general and clientSetKeepaliveFlag
> intent/documentation in particular. I do not know whether it introduces
> more bugs, but I would not be surprised if it does because the
> if-statements above the new code do not enumerate "all overriding reasons"!
>
> To add insult to the injury, the commit message is also misleading
> because, bugs notwithstanding, Squid did "send keep-alive unless we had
> a good reason to close" even before this change.
>
> Can we revert the above change, please?
>
>
> You may want to test the attached fix instead. I do not know whether it
> helps with Bug 2936 specifically, but it does fix a bug that smells
> related to those issues because Bug 2936 test script uses HTTP/1.0 messages.
>
> Thank you,
>
> Alex.
Received on Sat Aug 14 2010 - 07:52:03 MDT