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

Be less specific about urgency when behind an intermediary #1264

Merged
merged 3 commits into from
Sep 28, 2020

Conversation

LPardue
Copy link
Contributor

@LPardue LPardue commented Sep 17, 2020

Closes #1022

@LPardue
Copy link
Contributor Author

LPardue commented Sep 23, 2020

cc @martinthomson, since it closes your issue.

Comment on lines 545 to 548
the intermediary as another signal in its prioritization decisions. For
instance, if a server knows the intermediary is coalescing requests, then
it could prioritize responses using a fixed urgency level with incremental
loading. Treating all requests from an intermediary in this way would result in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why you would recommend incremental loading at all. You might just say "assign all requests the same priority". And then add the note about where the bottleneck is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this context, incremental means round-robin. We don't care how the thing receiving the response processes it. But we do want to avoid the server responding as FIFO, which might cause a different type of fairness problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Knowing that the intent of this PR is to be less specific, I think we can get rid of suggestions that refer to the specification (i.e., use of words like "urgency" and "incremental").

Rather, we can condense this sentence and the following one to something like: For instance, if a server knows that the intermediary is coalescing requests, it could serve the responses in round-robin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, thanks!

Comment on lines 549 to 550
round-robin scheduling. This works well when the network bottleneck exists
between the intermediary and the end client, as the intermediary buffers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that the network is always the bottleneck. Sometimes it is processing resources that are being prioritized.

This can work if the constrained resource is network capacity between the intermediary and user agent.

(End client == user agent?)

Comment on lines 553 to 554
dynamic urgency that more closely reflects request urgency, in order to have
weighted round-robin scheduling. This would result in less urgent responses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop the last two sentences entirely. Statements about sophistication will likely seem quaint in 6 months time.

Copy link
Contributor Author

@LPardue LPardue Sep 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about condensing the two to say "Weighted round-robin scheduling could improve the use of contained network capacity further."?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my preference goes to removing the last two sentences, due to the following reasons:

  • Use of WRR degrades fairness, and might have negative impact on perceived performance of an end user.
  • This is a problem that we do not need to solve now. Our choice of two priority parameters has been based on what we want to have for web browsing. For web browsing, we do not expect to see the case being described here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, thanks!

@LPardue
Copy link
Contributor Author

LPardue commented Sep 25, 2020

I believe this is now good to go, please approve or speak up if you disagree.

@kazuho kazuho merged commit 088a4ec into master Sep 28, 2020
@kazuho kazuho deleted the priority-coalescing branch September 28, 2020 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Coalescing intermediaries and fairness
3 participants