Subject: [quicwg/base-drafts] allow PRIORITY frames referring to placeholders exceeding `SETTING_NUM_PLACEHOLDERS` (#2761)

From: Kazuho Oku <[email-masked]>
Date: Thu, 30 May 2019 22:19:18 -0700

At the moment, it is the client's obligation to build a prioritization tree by respecting the server-advertised value of `SETTINGS_NUM_PLACEHOLDERS`.

That seems like an unnecessary complexity. We assume servers to provide sensible amount of space for placeholders (i.e., SHOULD). Then, why do the clients need to be _forced_ to implement complex logic to handle servers not complying to the recommendation?

This PR gives the clients the freedom to express a prioritization tree that it needs regardless of what the server advertises using the `SETTINGS_NUM_PLACEHOLDERS` settings. The settings is changed to a purely advisory value; clients that do care are allowed to respect the value.

Maybe closes #2734, #2753.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2761

-- Commit Summary --

  * allow PRIORITY frames referring to placeholders exceeding `SETTING_NUM_PLACEHOLDERS`

-- File Changes --

    M draft-ietf-quic-http.md (30)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2761.patch
https://github.com/quicwg/base-drafts/pull/2761.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 31 May 2019 02:18:51 -0700

@kazuho pushed 1 commit.

a87dd7a73047aa243ab943b3f0570a8cf382dc69  s/0/zero/

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761/files/231f8d9ecf7a13c76b809dcd704eef0683272391..a87dd7a73047aa243ab943b3f0570a8cf382dc69

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 03:17:18 -0700

LPardue commented on this pull request.

> @@ -1202,9 +1202,15 @@ The following situations are examples of invalid PRIORITY frames:
 A PRIORITY frame with Empty bits not set to zero MAY be treated as a connection
 error of type HTTP_MALFORMED_FRAME.
 
-A PRIORITY frame that references a non-existent Push ID, a Placeholder ID
-greater than the server's limit, or a Stream ID the client is not yet permitted
-to open MUST be treated as a connection error of type HTTP_LIMIT_EXCEEDED.
+A PRIORITY frame that references a non-existent Push ID, or a Stream ID the
+client is not yet permitted to open MUST be treated as a connection error of
+type HTTP_LIMIT_EXCEEDED.
+
+A PRIORITY frame MAY reference a Placeholder ID that is equal to or greater than
+the server's advertised value of the `SETTINGS_NUM_PLACEHOLDERS` settings.  The
+server SHOULD ignore PRIORITY frames that specify such placeholders as the
+prioritized element.  The server SHOULD use the root of the tree as the

I think you want s/prioritized element/element dependency. Such that if a stream is assigned as dependent on a placeholder ID greater than SETTINGS_NUM_PLACEHOLDERS, then it becomes dependent on root.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#pullrequestreview-244235054

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 03:20:42 -0700

NVM, you say that in the next sentence. 

Perhaps if you reorder the text to make the subject of each sentence come earlier, it will be easier to read it correctly.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#discussion_r289335339

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 31 May 2019 07:02:22 -0700

@kazuho pushed 1 commit.

4579ada05eba97b83e8eb9833ea013b70d30cc10  editorial

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761/files/a87dd7a73047aa243ab943b3f0570a8cf382dc69..4579ada05eba97b83e8eb9833ea013b70d30cc10

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 31 May 2019 07:03:48 -0700

kazuho commented on this pull request.

@LPardue Thank you for the comment. I've changed the flow of the sentences in 4579ada. PTAL.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#discussion_r289402690

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 08:34:23 -0700

That looks better now.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#discussion_r289441411

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 08:39:25 -0700

A general comment on this is that things might get racy depending on how things get resolved with the initial value of SETTINGS_NUM_PLACEHOLDERS. If the default is low and a SETTINGS frame would increase it, then the client that attempts to set placeholder priority too eagerly may not get the result they expect.

In practice what we might see is that different servers implement some internal limit to num placeholders and manage the tree that way, settling on some value that works well for most clients. 

I think that *is* the spirit of this change. This change allows some looser behaviour and unlike HTTP/2 phantom streams, doesn't tie up stream credits for prioritisation structures.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#issuecomment-497756420

---

From: Alessandro Ghedini <[email-masked]>
Date: Fri, 31 May 2019 11:37:20 -0700

This change doesn't make a lot of sense to me TBH. If the client is allowed to ignore the server setting, then there's no point to that setting. 

If the client can't adapt its prioritization scheme based on the number of placeholders allowed by the server and proceeds to use more placeholders, then it is likely to end up effectively disabling prioritization for the connection completely (if the server does set a limit it probably means it won't respect the priorities that use more than the placeholders allowed, otherwise it would set the limit to a very high value).

So in practice servers will just end up setting their internal limits and the setting to either whatever value popular clients need, making the setting meaningless, or still set it to a lower value effectively disabling prioritization with some clients.

Anyway, TBH at this point I'd rather spend time exploring the discussion about limiting or even removing placeholders as we talked about in London (including talking with HTTPbis) rather than try to hack around all of these problems.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#issuecomment-497817478

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 11:44:42 -0700

This change puts placeholders in the same camp as MAX_HEADER_LIST_SIZE, an advisory value. I also agree with your argument there, a limit that is ignored is kinda useless. 

What this change does allow is a fire and forget approach from the client. It gracefully fails because the client can't reliably validate that any prioritisationinformation was adhered to.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#issuecomment-497819963

---

From: Alessandro Ghedini <[email-masked]>
Date: Fri, 31 May 2019 11:53:17 -0700

Sure, but clients either care about priorities, in which case they should be built such that they can deal with whatever placeholders limit instead of risking having priorities silently ignored, or they don't, in which case they might as well not send priorities at all.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#issuecomment-497822760

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 31 May 2019 19:09:20 +0000 (UTC)

I guess one argument is that since prioritisation is a hint anyway, the clients are already able to handle being silently ignored.

However, to support your argument, it may be better to ignore all priority tree requests rather than fail e.g. half of them. And because the server doesn't know how many placeholders the client wants, it can never guess how many would be enough, ever.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2761#issuecomment-497827661