Subject: [quicwg/base-drafts] create codec streams only when necessary (#2090)

From: Kazuho Oku <[email-masked]>
Date: Sun, 02 Dec 2018 18:45:11 -0800

Now that #2038 is being merged, we have the possibility to avoid creating unnecessary encoder / decoder streams.

Avoiding creation them is beneficial for memory-constrained devices, because they can configure the QUIC transport stack to handle at most one unidirectional stream (in each direction) to support HTTP/3 _if_ the QPACK state forbids 1) peer's creation of the encoder stream when endpoint's `SETTINGS_HEADER_TABLE_SIZE:` is zero, and 2) peer's creation of decoder stream is delayed until the endpoint's creation of the encoder stream.

The PR clarifies the two requirements.

With the proposed changes, a memory constrained HTTP/3 stack that sends`SETTINGS_HEADER_TABLE_SIZE` of zero and never opens an encoder stream can be certain that the peer never opens an encoder stream or a decoder stream.

Note that the proposed change has minimal impact to existing implementations; regarding creation of the encoder stream, #2038 states that the stream cannot be used until SETTINGS is being received. This PR only merely clarifies that "use" includes the creation of the stream. The situation is the same for the decoder stream.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * create codec streams only when necessary

-- File Changes --

    M draft-ietf-quic-qpack.md (12)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2090.patch
https://github.com/quicwg/base-drafts/pull/2090.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/2090

---

From: Martin Thomson <[email-masked]>
Date: Tue, 04 Dec 2018 09:00:43 -0800

This change would really drive the complexity of my code up, and I'm not really seeing a positive.

-- 
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/2090#issuecomment-444175332

---

From: Mike Bishop <[email-masked]>
Date: Tue, 04 Dec 2018 10:24:36 -0800

MikeBishop approved this pull request.

I'm of split mind on this.  I see the benefit for small implementations, though I suspect that benefit is marginal.  

At first glance, this seems like an implementation can no longer create the streams and then hand them to the encoder/decoder implementations -- the stream has to get lazily initialized, not sending the type byte until the codec is ready to send something further.  This probably also means not claiming a Stream ID either (use in order!), which means that the Stream ID allotment might not be there when you do go to send something.  Or, if you do claim the stream and then use a higher-numbered unidirectional stream, this one gets implicitly opened but the type byte is never sent, so it sits in limbo at the receiver.

However, I think the key here is in waiting until SETTINGS is received.  The new SETTINGS text says that you can operate in constrained mode (no dynamic table) before receiving SETTINGS.  That would mean that the call to the codec providing the streams and the table size is a step after the encoder and decoder are already operational, rather than start-up initialization.  This change, then, is simply a check in that call to determine whether to pass streams to the codec at all.

Seen from that perspective, this seems more palatable.

>  Receipt of a second instance of either stream type MUST be treated as a
 connection error of HTTP_WRONG_STREAM_COUNT.  Closure of either unidirectional
 stream MUST be treated as a connection error of type
 HTTP_CLOSED_CRITICAL_STREAM.
 
+An endpoint MUST NOT create an encoder stream if the maximum size of the
+dynamic table permitted by the peer is zero.  Observation of a peer-initiated
+encoder stream when the value is zero MUST be treated as a connection error of

"without sending a non-zero value for `SETTINGS_HEADER_TABLE_SIZE`"

>  Receipt of a second instance of either stream type MUST be treated as a
 connection error of HTTP_WRONG_STREAM_COUNT.  Closure of either unidirectional
 stream MUST be treated as a connection error of type
 HTTP_CLOSED_CRITICAL_STREAM.
 
+An endpoint MUST NOT create an encoder stream if the maximum size of the
+dynamic table permitted by the peer is zero.  Observation of a peer-initiated
+encoder stream when the value is zero MUST be treated as a connection error of
+type HTTP_QPACK_ENCODER_STREAM_ERROR.
+
+An endpoint MUST NOT create a decoder stream until the peer opens an encoder
+stream.  Observation of a peer-initiated decoder stream that lacks the
+corresponding encoder stream MUST be treated as a connection error of type

"when no encoder stream has been initiated locally"

-- 
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/2090#pullrequestreview-181408669

---

From: Mike Bishop <[email-masked]>
Date: Tue, 04 Dec 2018 10:26:46 -0800

Minor process point:  this is a design change that really ought to have an issue.

-- 
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/2090#issuecomment-444205405

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 05 Dec 2018 00:49:32 -0800

@MikeBishop 
> Minor process point: this is a design change that really ought to have an issue.

My apologies. I've filed #2100.

-- 
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/2090#issuecomment-444406342

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 05 Dec 2018 00:54:57 -0800

@kazuho pushed 1 commit.

0ea3696ecd965ccb979e4723d9286a5f7481278a  editorial improvements suggested by @MikeBishop

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2090/files/3dac7a30d156077f7be63def2590a5f9904c1471..0ea3696ecd965ccb979e4723d9286a5f7481278a

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 05 Dec 2018 09:04:16 +0000 (UTC)

kazuho commented on this pull request.

Thank you for the suggestions. Applied in 0ea3696.

-- 
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/2090#discussion_r238978583

---

From: Luca Niccolini <[email-masked]>
Date: Wed, 05 Dec 2018 03:34:16 -0800

I stand with @martinthomson on this. I feel like optimizing the common usecase for constrained devices would significantly increase complexity of the implementation. 
Could we instead relax the constraint that qpack streams (being control streams) cannot be closed, and allow an endpoint that doesn’t want to do qpack to not open its control streams and to immediately reset (with a specific error code, yeah I know YAEC) the peer initiated qpack streams? 
On the endpoint receiving the reset this would be a connection error if the received table size was not zero. 

-- 
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/2090#issuecomment-444454787

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 05 Dec 2018 13:50:13 +0000 (UTC)

@martinthomson, @lnicco: from the current spec which states that QPACK can be used before _SETTINGS_ are received and the encoder and decoder streams are opened to @kazuho's proposal the distance is small: it is a single step.

As for your code: everyone's code is different. The complexity increase in _my code_ would be small. This weakens your argument.

This proposal is logical, it makes the spec clearer, and would help not only small devices, but also allow simpler HTTP/3 implementations to exist.

-- 
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/2090#issuecomment-444491329

---

From: Subodh Iyengar <[email-masked]>
Date: Wed, 05 Dec 2018 15:23:55 +0000 (UTC)

@kazuho I’m a bit confused of how this works with 0.5 rtt and default dynamic table size of 0. The server might open the qpack stream in 0.5 rtt, and then the client may advertise a size of 0. What would happen to this qpack stream. Or does this pr affect default table size as well

-- 
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/2090#issuecomment-444523677

---

From: Mike Bishop <[email-masked]>
Date: Wed, 05 Dec 2018 18:10:39 +0000 (UTC)

@siyengar, under the current text the server might open the stream in 0.5 RTT, but couldn't actually send anything beyond the type byte until it receives SETTINGS because of the default size of zero.  With this change, it wouldn't open the stream until it receives SETTINGS, and then only if the size were non-zero.

-- 
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/2090#issuecomment-444585529

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 Dec 2018 21:39:40 -0800

Let me try to expand on my earlier comment.  I currently create these streams when building my connection object.  This allows requests and responses to use those streams directly.  If the streams might not exist, then I have to allow those values to be absent throughout, which is annoying but not a big deal.  More of a concern is that this is concurrent code, so deferring creation would mean that I would have to guard the variables holding the streams with a mutex (or atomic, which always gives me the creeps).  That creates the potential for contention point on every new message.

Not saying impossible, but it is annoying.  And all in service to implementations that are not interested in handling a stream that will only ever contain a single octet.  Given the complexity of this protocol as a whole, it seems like a strange optimization to make.

-- 
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/2090#issuecomment-446468738

---

From: MikkelFJ <[email-masked]>
Date: Wed, 12 Dec 2018 10:12:36 +0000 (UTC)

I agree with MT that unnecessary concurrency is bad, but without digging into MT's architecture, I'd just point out that atomic reads or writes of integers are generally very cheap operations, unlike atomic exchange and the likes. This only applies when you can afford to react lazily to updates - such as more flow credits available. Technically you may need a costly bus sync (fence) to eventually see writes by other cores, but in praxis this happen soon enough.

-- 
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/2090#issuecomment-446533829

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 12 Dec 2018 19:35:59 -0800

@martinthomson 
> Let me try to expand on my earlier comment. I currently create these streams when building my connection object. This allows requests and responses to use those streams directly. If the streams mig…

Isn't it the case that you would be having contention point even without the proposed change, assuming that you are creating the connection object prior to receiving the SETTINGS frame? I ask this because the receipt of SETTINGS_MAX_TABLE_SIZE updates the state of the QPACK encoder.

-- 
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/2090#issuecomment-446831416

---

From: Martin Thomson <[email-masked]>
Date: Wed, 12 Dec 2018 20:03:43 -0800

I already have to manage contention for encoding, so I guess you are right, this isn't that big a problem.  It's just complexity with questionable benefit.

-- 
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/2090#issuecomment-446835432

---

From: afrind <[email-masked]>
Date: Tue, 18 Dec 2018 21:12:02 +0000 (UTC)

What if we let the memory constrained implementation prevent the creation of the encoder/decoder streams by limiting the number of peer initiated unidirectional streams, rather than requiring lazy creation?

-- 
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/2090#issuecomment-448372135

---

From: Dmitri Tikhonov <[email-masked]>
Date: Tue, 18 Dec 2018 13:19:43 -0800

That would rule out push promises.

-- 
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/2090#issuecomment-448374448

---

From: afrind <[email-masked]>
Date: Tue, 18 Dec 2018 13:54:11 -0800

@dtikhonov : yes, I guess I'm assuming that memory constrained implementations that don't really implement qpack aren't going to support push either.  And they are not going to support any extensions that require more than one unidirectional stream.  Per @kazuho's original problem statement:

> Avoiding creation them is beneficial for memory-constrained devices, because they can configure the QUIC transport stack to handle at most one unidirectional stream

Is anyone actually building a memory constrained implementation that would like to weigh in here?  It strikes me as pretty simple to just receive a qpack unidirectional stream and discard it, or as @lnicco suggests, reset it.  

My thoughts around implementing a simpler version of qpack was that you wouldn't be required to open your own encoder/decoder stream.

-- 
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/2090#issuecomment-448384835

---

From: Dmitri Tikhonov <[email-masked]>
Date: Tue, 18 Dec 2018 13:58:06 -0800

Resetting a QPACK stream would trigger an error:
> Closure of either unidirectional stream MUST be treated as a connection error of type HTTP_CLOSED_CRITICAL_STREAM.

-- 
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/2090#issuecomment-448386037

---

From: Luca Niccolini <[email-masked]>
Date: Wed, 19 Dec 2018 01:37:04 -0800

@dtikhonov yes with the current text, resetting any control stream is illegal. 
However given that we are discussing about making qpack streams kind of special, we _could_ allow resetting them. And that is what I proposed in my first comment.

> Could we instead relax the constraint that qpack streams (being control streams) cannot be closed, and allow an endpoint that doesn’t want to do qpack to not open its control streams and to immedia…

My original point about the complexity of implementation for little benefit seems to be same that others have, and leaving aside concurrency and implementation specific concerns (which I don't deny exists in our code, but they are easily fixed) my point was more general as I feel like we are trying to bake into the protocol an optimization that in my opinion is better handled in the implementation of the memory constrained devices and we should just make it possible, rather than adding more constraints to the common case. So generally speaking I feel more comfortable relaxing constraints rather than tightening them, if possible and if that does not pose a threat.

-- 
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/2090#issuecomment-448530612

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 19 Dec 2018 13:16:02 -0800

@afrind 
> What if we let the memory constrained implementation prevent the creation of the encoder/decoder streams by limiting the number of peer initiated unidirectional streams, rather than requiring lazy…

In addition to the reset issue, IIUC we do not currently require the endpoints to open QPACK codec streams before the control streams. We'd need to fix that, otherwise, there's a risk that endpoints that restrict the number of unidirectional streams would suffer multiple round-trips to exchange SETTINGS. However, requiring the streams to be in particular order (i.e. particular streams to have particular stream IDs) contradicts to our current approach of using the type byte rather than the stream IDs for identifying unidirectional streams.

-- 
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/2090#issuecomment-448746507

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 19 Dec 2018 21:57:04 +0000 (UTC)

@afrind 
> Per @kazuho's original problem statement:
> 

Admittedly, I referred to the case of memory-constrained devices. However by reading the comments, I start to wonder if that is the only use-case that would suffer from the unnecessary creation of QPACK codec streams.

IIUC, the argument against mandating the lazy creation of QPACK codec streams is that the codec streams are very likely to be used, and that we should optimize for the majority. The logic itself sounds fine, but it's based on the premise that QPACK would be used for the entire lifetime of H3. If we are to start using an alternative to QPACK together with H3, every one of us would be handling the creation of QPACK codec streams that are useless.

H3 is the HTTP binding for the first non-TCP transport that is going to see lots of HTTP traffic. Considering how long we've been using TCP, it would be natural to assume (or hope) that H3's lifetime would span over multiple decades.

Compared to that, QPACK's lifetime might be shorter. It's not due to an issue in QPACK, but it's due to what's being compressed might change. The HTTP Working Group is working on [Structured Headers](https://tools.ietf.org/html/draft-ietf-httpbis-header-structure), that introduces structs and typed elements to the header fields. That changes how things can be compressed. Instead of compressing each header field, I'd assume that we'd be compressing each element of the header field (e.g., `stale-while-revalidate=YYY` would have it's own entry). We'd be compressing not only strings but also integers, booleans.

Should we be confined to develop the compression method for Structured Headers (and other changes that we might want to make in the future) as an extension to QPACK? I do not think we have enough information to make that decision.

To summarize, eager creation of QPACK codec streams could become unnecessary complexity not only for memory-constrained endpoints but for other endpoints as well. And I do not see why we need to ossify H3 in that way, considering the fact that the draft nevertheless requires endpoints to change the QPACK codec state when they receive SETTINGS (see [Mike's comment](
https://github.com/quicwg/base-drafts/pull/2090#pullrequestreview-181408669)).

-- 
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/2090#issuecomment-448758145