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

Placeholder limits make for awkward clients #2734

Closed
rmarx opened this issue May 21, 2019 · 8 comments · Fixed by #2922
Closed

Placeholder limits make for awkward clients #2734

rmarx opened this issue May 21, 2019 · 8 comments · Fixed by #2922
Labels

Comments

@rmarx
Copy link
Contributor

rmarx commented May 21, 2019

When chatting a bit about how/when to create placeholders on the server on the slack, there seem to be two camps:

  1. Server creates all placeholders it advertises upon sending the SETTINGS frame (full up-front)
  2. Server creates the placeholder when it is referenced by the client (as-needed basis)

Argument for not doing 2 seems to be:

Lazy loading placeholders is a great path to under estimating required resource and getting DoS'd by clients

However, when not doing 2, you will probably limit yourself in the amount of placeholders you allow (e.g., not 100, but more like 10).

So the question becomes: what if those 10 are not enough for the client? Is every complex client required to support two schemes, one with a minimal amount of placeholders (e.g., let's say a server only supports 1 or none), and one where the server allows their needed amount? What about more dynamic schemes, where the placeholder count needed might not be known by the client up-front?

Some potential "solutions" to think about:

  • Specifying a minimal value for SETTINGS_NUM_PLACEHOLDER (e.g., 20?)
  • Client indicating the amount of placeholders they expect to need in their own SETTINGS, server deciding with some heuristics if it allows that
  • New option in which the client can articulate the full placeholder setup in a single packet instead of separate PRIORITY frames (though that doesn't cover the dynamic setup case above)

This may seem like a non-issue, but depending on the prioritization setup we end up choosing (see #2700, #2723, #2690, etc.) a larger amount of placeholders might be required to be able to model all use cases, and then clients really don't want to be limited in the amount they are allowed to use.

@dtikhonov
Copy link
Member

However, when not doing 2, you will probably limit yourself in the amount of placeholders you allow (e.g., not 100, but more like 10).

I don't understand what you mean. A server-advertised limit is just that -- why does it matter whether an implementation pre-allocates placesholders or creates them on demand?

@rmarx
Copy link
Contributor Author

rmarx commented May 21, 2019

It was more from my perspective that I would just declare SETTINGS_NUM_PLACEHOLDER to be something relatively large each time (e.g., 100+) and then just lazy-initialize as needed, to allow the client as much leeway as possible. If I actually have to create those 100+ up-front, I'm much less likely to allow that many placeholders, hence clients with complex setups might run into problems.

@LPardue
Copy link
Member

LPardue commented May 21, 2019

A server advertises a limit it should be able to fulfill that entire limit. For example, by reserving the capacity - even if the structures are not explicitly allocated. A naive implementation that advertises a large limit (like MAX_INT) and fails to reserve capacity shouldn't get surprised if things go bad when a client comes along and attempts to prioritise MAX_INT placeholders...

This is mostly an implementation detail.

The real points that we should focus on are on understanding if the limit unilaterally set by the server is the right approach given the way people might want to use placeholders.

@ianswett
Copy link
Contributor

My assumption was that clients that use placeholders need to have two options or a way to adapt one into the other.

I'd prefer an approach where existing clients don't need placeholders, so this isn't a pressing problem, but that would require Firefox to change their prioritization scheme and would create some problems if Chrome wanted to replace their linked list of streams with a linked list of placeholders as you've suggested elsewhere.

@ianswett ianswett added the -http label May 21, 2019
@ianswett
Copy link
Contributor

@MikeBishop noted that if there are 0 placeholders, a client could do the pre-placeholder approach of creating a tiny hanging GET or long standing request and then hang requests off that. Which is awful.

Hopefully that doesn't happen, but given placeholders are somewhat of an accident, some unintended consequences seem possible.

@afrind
Copy link
Contributor

afrind commented May 29, 2019

I prefer having a non-zero minimum that servers are required to support. If a client needs more than this minimum, it better have a fallback plan. I don't think this is too onerous on minimal implementations, since the server can just ignore all the PRIORITY frames anyways.

@kazuho
Copy link
Member

kazuho commented May 31, 2019

@afrind I think there are three options:

  • a) require some servers to lie (send non-zero SETTINGS_NUM_PLACEHOLDERS even though they do not respect placeholders)
  • b) require all clients to build different trees based on the value of SETTINGS_NUM_PLACEHOLDERS
  • c) let the clients send placeholders out of the server's advertised range

I do not think a is a good idea, because we'd be losing some signal. b is what we have now, but seems like an unnecessary complexity considering the fact that some servers might still ignore placeholders (or the priority tree as a whole). To me c seems to be a good middle ground. Clients that want to respect SETTINGS_NUM_PLACEHOLDERS can do so, but that becomes an option. Hence #2761.

@rmarx
Copy link
Contributor Author

rmarx commented Jun 5, 2019

As I wrote here #2761 (comment), I think we need a combination of c) and requiring servers to have a minimum amount of placeholders (even if they have to lie about it, e.g., in the IoT use case).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants