httpwg / http2-spec

Working copy of the HTTP/2 Specification
https://httpwg.org/
3.72k stars 565 forks source link

Cut Server Push #786

Closed martinthomson closed 3 years ago

martinthomson commented 4 years ago

@ianswett says:

It's not very widely used and many implementations either don't support it at all or don't support it in a way that improves end-user performance.

This sparked a lively discussion at https://github.com/martinthomson/http2v2/issues/16.

Lukasa commented 4 years ago

I’m open to dropping this, but I think the case needs to be be made for what the burden is. It’s one of the easiest protocol features to just ignore: send SETTINGS_ENABLE_PUSH=0 and then you can remove all awareness of PUSH_PROMISE from your state machine altogether.

mnot commented 4 years ago

Crazy thought: what if we made SETTINGS_ENABLE_PUSH=0 part of HTTP/2, and then factored server push into an extension?

Lukasa commented 4 years ago

Or what if we minted a new ALPN (#787) and flipped the default value?

grmocg commented 4 years ago

Or left it as is, since it is used outside of browsers. The browsers have an API problem that has not been solved, and it is unlikely that push will be effective there (except as a competitor to inlining) until that changes.

On Wed, Oct 28, 2020 at 1:28 AM Cory Benfield notifications@github.com wrote:

Or what if we minted a new ALPN (#787 https://github.com/httpwg/http2-spec/issues/787) and flipped the default value?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/httpwg/http2-spec/issues/786#issuecomment-717778783, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHV24RKH4P2UWKRFVI3BATSM7I4TANCNFSM4TB5OJ2Q .

MikeBishop commented 4 years ago

I have no particular objection to flipping the default under a new ALPN. Factoring it out to an extension is harder, but not impossible -- it's pretty embedded in the state machine, but we might be able to abstract that away. Removing it entirely seems challenging for several reasons. The purely process one is that RFC 8030 requires Server Push, and removing a feature used by a Standards Track RFC presumably requires deprecating that other RFC.

Lukasa commented 4 years ago

I should note that my preferred outcome here is "do nothing". I'm just spitballing ideas.

ianswett commented 4 years ago

Flipping the default if we mint a new alpn seems sensible based on the comments above. This would also align with HTTP/3.

Moving it to an extension is probably more work than it's worth, I'll admit.

I wasn't aware of RFC 8030. Is that an actively used RFC?

bagder commented 4 years ago

Push is being used outside of browsers (curl supports it, once implemented for a particular company that uses/used it pretty widely across the globe), although I suspect that outside of browsers we also have no real means of figuring out exactly how much or little.

Saying "do it in extension" will probably equal killing it, but I also think that removing it from the spec is more work than its worth and just toggling the default seems like the best middle ground.

Lukasa commented 4 years ago

We may want to be a bit relaxed about flipping the default. As the setting is client-only the default value never really exists on a connection: the client’s SETTINGS are the first frame it sends. The reason to flip the default is to save 6 bytes on all connections. This is not nothing, but it’s not a huge win either.

wtarreau commented 4 years ago

I agree that it's not worth changing anything here, except if we want to be sure to cause some confusion between implementations. The first frame sent is the one indicating support (or lack thereof) by the client.

Maybe we should just add a paragraph in the PUSH section mentioning that 5 years after the relerase, PUSH has still got very low adoption among implementations and should not be assumed as a granted feature for protocols sitting on top of H2.

ianswett commented 4 years ago

I think @wtarreau suggestion of adding a note about deployment and not expecting wide support would be very helpful. My biggest concern with keeping it in the spec is that people will expect it's widely supported and its use is recommended for most use cases.

mnot commented 4 years ago

Feeling here seems to be supportive of no normative changes, but possibly some editorial additions clarifying the status of push. If you feel otherwise, please say so soon.

ianswett commented 4 years ago

SGTM.

FYI, Chrome is expected to drop support for HTTP/2 server push in the= near future and has never implemented push for HTTP/3.

GregoireHebert commented 4 years ago

@ianswett has it been announced somewhere? I can't find it.

dunglas commented 4 years ago

Several popular web frameworks actively use Server Push including Symfony (mostly for assets) and API Platform (for API relations). Server Push is also (currently) supported by most browsers as well as by NGINX, Apache and Caddy.

On the APIs field, the Vulcain spec, which aims to replace GraphQL-like document compounding is also gaining adoption, and rely on Server Push (even if it can fallback on preload Link headers and Early Hints too).

I just published a benchmark showing how useful Server Push is for the specific use case of web APIs. Under certain conditions, relying on it can be 4x times faster than using compound documents: https://github.com/dunglas/api-parallelism-benchmark

As pointed in martinthomson/http2v2#16, for these use cases, Server Push could be replaced by Early Hints or maybe by something using WebTransport, but these specs aren't currently implemented while Server Push is broadly available.

Removing support for Server Push in Chrome will hurt all this use case.

What's really missing with HTTP/2 Server Push is a way to prevent pushing resources already stored by the client. Cache Digests for HTTP/2 could have been a solution, but the work on it looks stopped.

ianswett commented 4 years ago

https://groups.google.com/a/chromium.org/d/msg/blink-dev/K3rYLvmQUBY/vOWBKZGoAQAJ

Lukasa commented 3 years ago

@ianswett suggested calling out the current status: lack of support in browser APIs for push, pointy edges in the usage, and the limitations in the spread of its usage. I think this is a reasonable editorial change.

Lukasa commented 3 years ago

@mnot to draft a document to encourage users to set the value to 0 by default and move on.

mnot commented 3 years ago

Discussed at Feb 2021 interim; intent to document concerns / caveats, recommend sending setting = 0. @mnot to PR.