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