[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: Greg Stein <gstein_at_gmail.com>
Date: Sat, 22 Jun 2013 19:39:23 -0400

On Sat, Jun 22, 2013 at 5:17 PM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> On Sat, Jun 22, 2013 at 11:12 AM, Lieven Govaerts <svnlgo_at_mobsol.be> wrote:
>> On Sat, Jun 22, 2013 at 10:24 AM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> ...
>>> Maybe I'm missing something, but why would you downgrade to HTTP/1.0
>>> only because of the 411 (Content Length Required)? Can't you just
>>> continue using HTTP/1.1, but use CL instead of chunked requests? Or is
>>> falling back to HTTP/1.0 just the quickest solution within the current
>>> implementation?
>>
>> There are only 2 differences between 'http 1.0 mode' and 'http 1.1
>> mode' in ra_serf. First is using Content-Length instead of chunked
>> encoding. The second is that the Connection:keep-alive header is set
>> on all responses.

Hmm. We should probably stick with the Keep-Alive even on 1.0
requests. Some proxies understand that.

>> I actually expected a third difference, pipelining should be disabled
>> in http 1.0 mode via set_max_outstanding_requests call, but I don't
>> see that in the code. gstein?

HTTP/1.0 can do pipelining, so I see no reason to disable that.

>>> IIUC HTTP/1.1 has other advantages besides chunked encoding, and can
>>> be used perfectly well without it. So it seems you can fall back to
>>> HTTP/1.1 + ContentLength here, no?

Right. We could use compression, for example. (iirc, 1.0 doesn't have that)

>> Yes, that's indeed possible. It does add yet another variant of
>> communication though. IMO we already have more options than we can
>> support, we should cut down in the multitude of ways we support legacy
>> systems.

I'm thinking we just have two flags: http10 (aka "don't know whether
the server is 1.0 or 1.1") and "disable_chunked_requests" (or some
shorter name).

>...
>>>> This will add one round-trip of overhead for every serf session, and
>>>> it impacts all users, even those that are not behind such (outdated)
>>>> proxies.

If you send a second request, then I'd suggest just repeating the
OPTIONS rather than a PROPFIND. It will be much easier on the server.

>>>> I'd support making this dependent on a 'http1.0_server" option in the
>>>> servers runtime configuration. With this option it only impacts those
>>>> users that are working behind outdated proxies. We can include the
>>>> necessary instructions for users in the error description of the
>>>> by-default-unexpected 411 response.
>>>>
>>>> This should also be a clear trigger for hosting companies to upgrade
>>>> their proxy infra.

The flag could be something like "busted-proxy = on". When you start
seeing blog posts like "In order to checkout from EXAMPLE.COM, you
must set busted-proxy=on in your .subversion configuration", and that
gets back to EXAMPLE.COM ... you can bet they'll dig into the problem
and fix it :-)

[ as an aside, I complained to an svn hosting company that their proxy
config broke serf; they were in transition and accidentally busted it;
they had it fixed a few days later ]

I would be in favor of instructions on how to fix these 411 errors,
then have the user set the flag and try again.

Now: the failure mode when the server *does* fix the flag, is a second
OPTIONS is sent when it doesn't need to be. Not a horrible problem.

>>> I'd favor always auto-detecting, even at the cost of the extra
>>> round-trip. Or perhaps make the default auto-detection, and have a
>>> flag to shortcut the auto-detection if you know what you're doing.
>>
>> That'd be the 'http1.0_server' flag, or the cached flag I've mentioned
>> in my other mail.
>
> I'm not really fond of a runtime config flag for this, but then again
> I'm also not fond of forcing the extra roundtrip for all users on the
> account of these wonky proxies (which I suppose are really a small
> minority).

We should *not* send an extra request, unless the user specifically
instructs us to. In a properly-working environment, it would not be
needed. That is our typical and ideal situation. Users/admins will
need to compensate for their broken setup. It isn't really our
problem, but we'll help a little (a flag) for them to work through it.

> The cached flag sounds to me like a risky solution (in the sense that
> it may become a source of bugs and strange behavior). What if a proxy
> gets upgraded (or configuration changed, for better (enabling chunked
> encoding) or for worse (disabling it))? How will the cache be
> invalidated? If it's not invalidated in some automatic way, a client
> that has "use 1.0" cached for a particular server will continue using
> HTTP/1.0 forever, even if the server / proxy gets upgraded, ... If
> it's the other way around, the user will suddenly start getting errors
> (so the error message still better point out the "manual override" or
> "clear cache" switch).

I'd suggest runtime checking (a second OPTIONS) based on a user
setting, or a cached flag. If we find that the host suddenly starts
supporting chunked requests, then we can clear the cache. We could
even warn the user "you set busted-proxy, but it appears to be working
now; you may want to clear that flag".

> At the moment I don't have any better ideas though. Perhaps we / you
> should wait a little more for other people to give their opinion or
> suggestions about this?
>
> How about Kevin's suggestion [1], that serf should ideally be using CL
> instead of chunked-encoding for the requests?

As a default? Absolutely not.

In a future serf where we change the bucket model to include a
"getsize" method, then we could use C-L when the bucket reports a
known/fixed size. Some buckets may not be able to do that, of course,
and we defer to chunked requests.

Ideally, ra_serf should be implementing some custom buckets,
corresponding to different types of requests it makes. We could
dynamically build the request, which would (typically) be much more
efficient (on-demand/lazy reading and lower memory overhead).

Cheers,
-g
Received on 2013-06-23 01:39:55 CEST

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