Runtime: HTTP2: Determine how to enable HTTP2 for SocketsHttpHandler

Created on 6 Feb 2019  路  24Comments  路  Source: dotnet/runtime

Currently, we only support HTTP2 in SocketsHttpHandler via opt-in usin gAppContext or environment variables. See dotnet/runtime#26991.

We need to determine how to expose this via public API.

api-needs-work

All 24 comments

Why add another API for this? We can simply change the default from opt-in to opt-out on the current AppContext and environment variable mechanisms.

Have we closed on that being the plan? I thought the plan was still to make it opt-in somehow.

And also, if that is the plan, what's the model for opt-out? AppContext/env vars? Public API?

I thought the plan was still to make it opt-in somehow.

We already have an opt-in plan. It's the current AppContext and environment variables.

If this issue is about adding a new public API to do the same thing, the question then is why? Would the API be instance or static on HttpClient and/or HttpClientHandler. Adding instance APIs to those objects, for example, means that library consumers can't control it (i.e. they might not create the HttpClient directly). The current AppContext or environment variable approach gives people control over HttpClient's they don't create.

And also, if that is the plan, what's the model for opt-out? AppContext/env vars? Public API?

If we assume that the majority 80% case is to just let the overall default be HTTP/2, then we just switch the opt-in to opt-out. Developers needing precision control can always set the HttpRequestMessage.Version to be (1,1) instead of the default (2,0) which gives them control per request.

From what I've seen, HTTP/2 for the scenarios we target doesn't add a whole lot of value, and adds overheads (e.g. synchronization costs to multiplex onto the same connection), which makes me question a decision defaulting to HTTP/2. In fact, I question our previous decision to change the default HttpRequestMessage version to be 2.0.

Here's my suggestion:

  • We enable SocketsHttpHandler to respect HttpRequestMessage.Version == 2.0, no environment variables, no appcontext, etc. (unless we're not confident in corner cases of the implementation, in which case we could have an environment variable / appcontext switch to disable it, as we did when SocketsHttpHandler was introduced). If the request asks for 2.0, SocketsHttpHandler tries to use it.
  • We revert our previous decision and change HttpRequestMessage.Version back to being 1.1. By default all requests use 1.1 unless they opt in to 2.0 by setting HttpRequestMessage.Version to it. This change was only made for .NET Core, and it won't affect the vast majority uses on Core, where SocketsHttpHandler is being used by default and doesn't support 2.0 at all in shipping bits, so they're already using 1.1 in the vast majority of cases; for everyone this does affect (who opted in to using another handler), it's a major version release, we ship side-by-side, you can opt-in by setting the message version, etc.
  • This addresses the case of anyone manually creating an HttpRequestMessage. But HttpClient also manufactures HttpRequestMessages implicitly as part of some of its helper methods. So, we add an instance property to HttpClient public Version DefaultMessageVersion { get; set; }. If null (the default), the default version on the message is used, so 1.1. If non-null, when HttpClient constructs a message, it uses DefaultMessageVersion as the HttpRequestMessage.Version. So if you want to opt-in all of the GetXxAsync helpers to using 2.0, you just set this one property and you're done.
  • (Alternatively to the last bullet, we instead add an HttpRequestMessage.DefaultVersion static property, that defaults to 1.1, and allow it to be set to 2.0. Then everyone who constructs an HttpRequestMessage, whether it be the developer explicitly or HttpClient implicitly, gets an HttpRequestMessage with Version == DefaultVersion at that time. I don't like this as much because of the global impact of the change, but it's viable.)

I question our previous decision to change the default HttpRequestMessage version to be 2.0.

Overall I like your plan above. However, changing the default HttpRequestMessage.Version property to be (1,1) again will affect app-compat (source breaking not binary breaking) especially for UWP(UAP) builds. Perhaps not a big deal but we actually changed the default HttpRequestMessage.Version property to be (2,0) for UAP only long before we changed it for netcoreapp (because the WinRT version supported it). However, for UAP, we also ended up making WinRT specific properties for handling this in a different way, HttpBaseProtocolFilter.MaxVersion. That property was added to WinRT HttpClient APIs because the WinRT version itself had no HttpRequestMessage.Version property at all.

I want to point out that this is going to get messier with HTTP/3 and QUIC.

Unlike HTTP/2, HTTP/3 is not a seamless transition. The way you "discover" that a server supports HTTP3 is either
(a) Issue an HTTP/1.1 or HTTP/2 request, and receive a response with an Alt-Svc header on it
(b) You have a priori knowledge that the server supports HTTP/3

For (a), we can discover this and transition to HTTP/3 for future requests. For (b), we may want to add some sort of "assume server supports HTTP/3" setting at the handler level. It's not clear how useful this actually is, so perhaps it's not worth doing.

Another way to look at this is that HTTP/2 and HTTP/3 are not simple "new protocol version" in the traditional sense. You aren't just opting in to some new features or improved performance; you're actually using a quite different protocol that has different perf characteristics (both good and bad) and different discovery mechanisms.

So perhaps treating these as just new protocol versions isn't really the right approach.

Thanks, @geoffkizer. Maybe I'm just reading into it what I want to read, but to me your info / good arguments lends weight to https://github.com/dotnet/corefx/issues/35138#issuecomment-462531816, since it makes it so that everything is 1.1 by default, and you can opt-in as desired.

@stephentoub Yes, I agree we should make 1.1 the default.

The other thing I wonder about is whether it makes sense to treat these as a linear version. For example, it could make sense to prefer HTTP/3 but use HTTP/1.1 as a fallback. Or it could make sense to require HTTP/3 only, no fallback -- because you know the server supports HTTP/3. Or it could make sense to require HTTP/2 only, no fallback -- this is effectively what GRPC will require (edit: at least currently; perhaps it supports HTTP/3 in the future too). Not sure how a user would specify any of this. But I think it's worth thinking about more.

@stephentoub you mentioned you will push it forward, assigning to you. Thanks!

@stephentoub

I've some thinking on this and I have the following proposal.

  1. Revert HttpRequestMessage.Version so that the default value is new Version(1, 1)

  2. Remove AppContext and Environment Variable stuff we added to opt-in for HTTP/2.0

  3. Remove any other internal fields in SocketsHttpHandler to enable HTTP/2.0.

  4. Change SocketsHttpHandler to always observe the incoming HttpRequestMessage.Version.

  5. In order to help developers that use HttpClient convenience methods like GetAsync (that don't take an HttpRequestMessage) and for developers that don't actually create the HttpClient at all, add a new static property to HttpRequestMessage that will set the default Version field for all newly created HttpRequestMessage objects.

```c#
namespace System.Net.Http
{
public class HttpRequestMessage
{
// Existing APIs...

    // New API
    public static System.Version Version { get; set; } // default value is "new Version(1,1)"
{

}
```

cc: @dotnet/ncl

Thanks, David. That's exactly the same as my proposal, isn't it?

Ah, your (5) is different: I suggested an instance HttpClient.DefaultMessageVersion, and you're suggesting a static HttpRequestMessage.DefaultVersion. Why is the latter preferable?

If you put the static property on HttpClient, then it really should only affect HttpRequestMessage objects that are created by HttpClient itself which means it only affects the HttpClient convenience methods like GetAsync, PostAsync. It won't affect any HttpRequestMessage objects that are passed in.

I thought perhaps it would be more useful to just have a way to set the default for any HttpRequestMessage object that is created whether it was done via new HttpRequestMessage() by the developer or implicitly via HttpClient creating it.

If we think we only want to control the implicit HttpRequestMessage objects created by HttpClient, then I would suggest the property be called, "HttpClient.DefaultRequestMessageVersion" or HttpClient.DefaultRequestVersion".

I've been saying that the min bar here is:

  • Revert the default version back to 1.1, and
  • Either delete the env var / app ctx switch, or invert them (defaulting to on, letting you opt-out).

Sounds like we're all in agreement on that. I'm going to make that change.

On top of that (not blocking the above), there may be a desire to have APIs for controlling the version employed on the message at a higher-level than just setting it onto each HttpRequestMessage. We've discussed two options here. They're not mutually exclusive.

  1. Instance HttpClient.DefaultRequestVersion. If we do one thing, I think it should be this. HttpClient is already just a thin wrapper around a handler, with properties that impact how it creates the HttpRequestMessage that it then sends along to the handler, and this DefaultRequestVersion would be just like that, e.g. DefaultRequestHeaders. Those properties are instance, as well, controlling everything created by the HttpClient itself. And it means that you only impact things created by the instance you already hold, not by any other HttpClient that may have been created elsewhere in the process.

  2. Static HttpRequestMessage.DefaultVersion. We could do this in addition to (1), but it makes me nervous, having such a static configuration that one component might set and inadvertently end up impacting other components in the same process that are now unwittingly opted-in to use HTTP/2. It's not fine-grained enough to be the only such API, and I worry it's too coarse-grained to actually be useful.

I opened https://github.com/dotnet/corefx/issues/37224 for (1).

(1) is fine for now.

But it does not address the issue with consumers of libraries where the library is building the HttpClient. But we could assume that library producers will eventually start using this instance property of HttpClient (DefaultRequestVersion) and provide their own API to callers of the library.

But it does not address the issue with consumers of libraries where the library is building the HttpClient.

I would be very skeptical about forcing a library on which I depend to change its behavior in a way it wasn't tested for.

How do we plan to update our tests? I know that may not drive API discussion - but it is one use case at least.

The property discussion seems to depend on how aggressively we want to push use of HTTP2. HttpRequestMessage.DefaultVersion seems more convenient and HttpClient.DefaultRequestVersion seems to give more control.

How do we plan to update our tests?

I'm fixing them now.

How do we plan to update our tests? I know that may not drive API discussion - but it is one use case at least.

In terms of our tests, we already have a helper method "CreateHttpClient". So we can use that with whatever API (instance method) shape we use for turning on HTTP/2 for HttpClient convenience methods.

Note, as a library developer I need to alternate between WinHttpHandler and SocketsHandler to enable http/2 for net core 2.1+ (BTW, thanks for regressing this in 2.1!) I would strongly prefer to not introduce new 3.0 target and could rather determent at runtime if SocketsHandler support http/2 and I do not need to fallback to WinHttpHandler anymore.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noahfalk picture noahfalk  路  3Comments

btecu picture btecu  路  3Comments

Timovzl picture Timovzl  路  3Comments

v0l picture v0l  路  3Comments

chunseoklee picture chunseoklee  路  3Comments