Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove PRIORITY #2922

Merged
merged 2 commits into from Aug 27, 2019

Conversation

@martinthomson
Copy link
Member

martinthomson commented Jul 22, 2019

In all the discussions we have had on this, what has become clear is
that the complexity of the scheme we built in HTTP/2 wasn't well
founded in theory or practice.

Rather than try to port that across, this recognizes that while
prioritization is important, signaling is a problem we haven't solved
yet.

Closes #many issues.

EDIT:
Closes #2924
Closes #2753
Closes #2740
Closes #2739
Closes #2734
Closes #2697

@mikkelfj

This comment has been minimized.

Copy link
Contributor

mikkelfj commented Jul 22, 2019

Does this mean it goes away in v1, or restarting from scratch?

@kazuho

This comment has been minimized.

Copy link
Member

kazuho commented Jul 22, 2019

I think that this is the correct thing to do.

At the very least, I'd assume that server-driven prioritization using content-type would work well to some extent. Since the early days of H2, we've doing that for browsers that does not use dependency, and it is my understanding that it has been working OK. Though it would be preferable to have the capability of servers and clients providing "hints", especially to distinguish between a render-blocking JavaScript file and a asynchronously-loaded one.

Does this mean it goes away in v1, or restarting from scratch?

I'd assume that once the QUIC and HTTP working group agree to drop priorities from H3, it would become the task of the HTTP WG to determine what to do.

Copy link
Contributor

ianswett left a comment

Thanks!

@kazuho
kazuho approved these changes Jul 22, 2019
Copy link
Member

LPardue left a comment

I think this is in general a positive move. It closes some things out. However, I do wonder if it might make us revisit other things that were previously put to bed. For example, we might want to follow up the discussion about separate unidirectional streams [1], because the space is now smaller.

[1] #2678

({{request-response}}). Other frame types like SETTINGS, PRIORITY, and GOAWAY
are used to manage the overall connection and relationships between streams.
({{request-response}}). Other frame types like SETTINGS and GOAWAY are used to
manage the overall connection and relationships between streams.

This comment has been minimized.

Copy link
@LPardue

LPardue Jul 22, 2019

Member

A minor contention: IMO SETTINGS or GOAWAY don't exemplify "relationships between streams". In fact, removing tree depencies helps makes request streams way more independent.

This comment has been minimized.

Copy link
@martinthomson

martinthomson Jul 22, 2019

Author Member

Do you think that I should just strike that sentence?

This comment has been minimized.

Copy link
@LPardue

LPardue Jul 22, 2019

Member
Suggested change
manage the overall connection and relationships between streams.
manage the overall connection.

yes, or do this.

@janaiyengar

This comment has been minimized.

Copy link
Contributor

janaiyengar commented Jul 22, 2019

Please create an umbrella issue for discussion. I would like to go in this direction, but this requires discussion in the wg and in HTTP as well, and I would rather not have that on this PR.

@LPardue

This comment has been minimized.

Copy link
Member

LPardue commented Jul 22, 2019

I'm also a little nervy that the removal of text makes it appear that the first user of streams (HTTP/3) quietly ignores the transport's recommendation in section 2.3

A QUIC implementation SHOULD provide ways in which an application can
indicate the relative priority of streams. When deciding which
streams to dedicate resources to, the implementation SHOULD use the
information provided by the application.

@mikkelfj

This comment has been minimized.

Copy link
Contributor

mikkelfj commented Jul 22, 2019

I'm also a little nervy that the removal of text makes it appear that the first user of streams (HTTP/3) quietly ignores the transport's recommendation in section 2.3

That is an endpoint to API thing - it just says an H/3 impl. should be able to prioritize streams server side, e.g. from some configuration. It doesn't say two endpoints should negotiate this.

@LPardue

This comment has been minimized.

Copy link
Member

LPardue commented Jul 22, 2019

Fair point. I think the transport text is actually pretty weak in articulating what it is asking for in terms of stream prioritization. For example, it mentions exchanging information and then jumps to local-API concerns

QUIC does not provide a mechanism for exchanging prioritization
information. Instead, it relies on receiving priority information
from the application that uses QUIC.

@martinthomson martinthomson referenced this pull request Jul 22, 2019
@rmarx

This comment has been minimized.

Copy link
Contributor

rmarx commented Jul 22, 2019

I understand the appeal of this proposal, but I don't agree that simply cutting priorities without replacing them with something else is the right way forward.

I don't see how this PR actually solves anything... it will allow us to ship HTTP/3 sure, but it will be (even more) difficult to use correctly in practice. This puts the burden of figuring out prioritization fully on the servers, and we've seen how well that will go in practice with H2.

I would also like to see some more detailed explanation on the statement in the httpbis session just now (which was a bit more strongly worded than the original post in this thread):

prioritization is important but we don't need signaling for prioritization

I'm not sure I follow this... Are we suddenly claiming that the server does not need client-side information anymore, which is the opposite of the reasoning behind H2's setup? Do you feel you can do good server-side prioritization without client-side signaling? What would that look like? Combining content-type with user-agent? While I might agree with Kazuho that could work in the general case (#2922 (comment)), I think it would cause severe degradations for pages using more advanced features (e.g., as he mentions async/defer), which is ironic, as those are marketed as being good for performance.


In essence, I feel that if we remove tree-based priorities, we must endorse another solution (e.g., patrick meenan's setup, kazuho's http-header based solution) and standardize that alongside H3. I fear not doing this will lead to a mess that will be difficult to recover from (i.e., if we standardize the signaling part later on, it will be difficult to find enough uptake, which was one of the main goals on Ian's slides).

@LPardue

This comment has been minimized.

Copy link
Member

LPardue commented Jul 22, 2019

Do you feel you can do good server-side prioritization without client-side signaling? What would that look like? Combining content-type with user-agent?

Our experience shows that this is effective. It even improves performance for certain User Agents that use a poor prioritization scheme.

@kazuho

This comment has been minimized.

Copy link
Member

kazuho commented Jul 22, 2019

@rmarx

In essence, I feel that if we remove tree-based priorities, we must endorse another solution (e.g., patrick meenan's setup, kazuho's http-header based solution) and standardize that alongside H3.

I also think we should have a prioritization scheme before or when we deploy H3 in the wild, but process-wise, I think we should separate the discussion so as to minimize the chance of QUIC standardization process getting delayed, and because server-driven prioritization can work as a backup strategy.

@LPardue

This comment has been minimized.

Copy link
Member

LPardue commented Jul 22, 2019

devli's advocate: H3's inherent prioritization scheme can use request generation order (stream ID), a server uses this to either FIFO or round-robin. This is NOT optimal but is it disastrous enough that we absolutely should not be shipped that way?

@janaiyengar

This comment has been minimized.

Copy link
Contributor

janaiyengar commented Jul 23, 2019

Can we please have this discussion on the issue #2924, and not on this PR?

In all the discussions we have had on this, what has become clear is
that the complexity of the scheme we built in HTTP/2 wasn't well
founded in theory or practice.

Rather than try to port that across, this recognizes that while
prioritization is important, signaling is a problem we haven't solved
yet.

Closes #many issues.
Closes #2924.
@MikeBishop MikeBishop force-pushed the priority-diediedie branch from 5c18a9e to 3c1855d Aug 27, 2019
@MikeBishop MikeBishop merged commit a0c3d6e into master Aug 27, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
facebook-github-bot added a commit to facebook/proxygen that referenced this pull request Oct 10, 2019
Summary:
quicwg/base-drafts#2922
quicwg/base-drafts#2924

Reviewed By: mjoras

Differential Revision: D17835762

fbshipit-source-id: b972d08a117de0661b8dce288ac34227ddcd2b2e
@juliens juliens referenced this pull request Oct 12, 2019
1 of 28 tasks complete
fcharlie pushed a commit to fcharlie/quiche that referenced this pull request Oct 22, 2019
This setting identifier has been removed from the draft at
quicwg/base-drafts#2922.

gfe-relnote: n/a, change to QUIC v99-only code.  Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 272717598
Change-Id: I49da04545c3dcbe1afd95dcf41845fc0ab6bb04e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.