[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: Johan Corveleyn <jcorvel_at_gmail.com>
Date: Sat, 22 Jun 2013 23:17:23 +0200

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.
>
> 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?
>
>> 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?
>
> 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.

Okay, fair enough. You're the one doing the work :-), so I have little
argument that you should make it more complex than necessary. The most
important thing is that we can handle these weird acting servers/proxies at all.

>>> 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.
>>>
>>> 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.
>>
>> 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).

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).

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?

[1] http://svn.haxx.se/dev/archive-2013-06/0518.shtml

--
Johan
Received on 2013-06-22 23:18:13 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.