[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Wed, 26 Jun 2013 01:03:10 +0400

On Tue, Jun 25, 2013 at 10:45 PM, Greg Stein <gstein_at_gmail.com> wrote:
> On Tue, Jun 25, 2013 at 11:55 AM, Philip Martin
> <philip.martin_at_wandisco.com> wrote:
>> Branko ─îibej <brane_at_wandisco.com> writes:
>>
>>> I'm really not a fan of this config knob. Anyone who carries their
>>> laptop around will effectively have to set this as the default, because
>>> you never know when the next weird proxy will pop up in front of your
>>> server. And disabling chunked requests by default is a lot worse than
>>> the extra non-pipelined request for broken proxies, IMO.
>
> Right.
>
> Though I suspect most of the problems are reverse proxies in front of
> a particular server, so you can put the config option into a [server]
> config block instead of global. That will help to limit the problem,
> but lack of dynamic detection is still a problem.
>
What is the benefit of dynamic detection enabled by some knob in config file?

>> I suppose there might be a proxy that accepts some chunked requests but
>> not others and so we might get a 411 in the middle of a pipeline. For
>
> I doubt you would ever get a 411 in the *middle* of a series of
> pipelined request. The proxy is going to allow all, or allow none of
> the requests to use chunking.
>
> (or more precisely: I believe the code should make that assumption
> until we find/learn otherwise)
>
>> that case we probably need the config knob even if we add some automatic
>> detection. But that's a theoretical problem, the bug reports so far
>> involve a proxy with no support for chunked requests for which automatic
>> detection should be possible.
>
> Right. I believe the config knob should be "busted-proxy" and it
> should kick off a second OPTIONS request to detect whether chunked
> requests are allowed.
>
> To that end, I find r1496470 to be insufficient. We need the dynamic
> stuff, and we need to fix the configuration option name. I'm happy to
> work on these two pieces, if we have a bit of consensus.
>
I agree that option name can be improved. Just change it if you'd like.

Concerning dynamic stuff: I think it's better to add get_length()
method to serf_bucket and always send Content-Length. This solve all
potential issues with almost zero cost. In ra_serf we use
aggregate+simple buckets for XML bodies and file bucket for commit
(PUT) requests.

But this is not what can be implemented in svn 1.8.1 IMHO. That's why
I made very simple fix.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2013-06-25 23:04:01 CEST

This is an archived mail posted to the Subversion Dev mailing list.