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

Separate QPACK streams #1121

Closed
martinthomson opened this issue Feb 21, 2018 · 9 comments
Closed

Separate QPACK streams #1121

martinthomson opened this issue Feb 21, 2018 · 9 comments
Labels
-http -qpack design An issue that affects the design of the protocol; resolution requires consensus.

Comments

@martinthomson
Copy link
Member

QCRAM doesn't need to use the control stream for its activities. I propose two streams:

  1. A stream for header updates.

  2. A stream for header block acknowledgments.

These could probably be arranged as a bidirectional stream that is initiated by the encoding side with header updates on the send direction and acknowledgments in the other direction. However, the timing of creation of these streams suggests that unidirectional streams are better. It is possible that header blocks would need to be acknowledged before the encoder commits any changes to the header table, and having the acknowledgments blocked on the creation of a remote stream would add unnecessary delays.

This separation allows for several advantages and I can't see any significant downsides:

  • Framing overheads can be removed entirely (or significantly reduced). Neither stream requires any framing in practice (header table updates can be a contiguous stream of changes), and acknowledgments can be a continuous sequence of integers.

  • An encoder can be given exclusive use of a write stream for sending header updates and a read stream for receiving acknowledgments. A decoder can be given exclusive access to the reciprocal streams.

  • More streams means less unnecessary blocking. Though there is generally a cost to more streams, these streams have no interaction with other control stream functions (SETTINGS, PRIORITY, etc...). Increased independence means fewer chances for head-of-line blocking.

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -http -qpack labels Feb 21, 2018
@LPardue
Copy link
Member

LPardue commented Feb 21, 2018 via email

@afrind
Copy link
Contributor

afrind commented Feb 21, 2018

We could reserve certain stream numbers for these streams in the HTTP mapping, or we could have some kind of magic as you suggest to identify the purpose of the stream.

I think you would need some framing on the header update stream to define the boundaries where the decoder would send acknowledgements. It's also somewhat simpler to not start parsing instructions when the data might end in the middle of an instruction.

@martinthomson
Copy link
Member Author

You don't send acknowledgments to header updates. No need.

I prefer the magic number at the start of the stream. Then I don't need to build a stack that knows about stream identifiers (see also #910, which I support in full).

@MikeBishop
Copy link
Contributor

MikeBishop commented Feb 22, 2018

@afrind, this ties to @martinthomson's observation that BLOCKING is unnecessary. Rather than making the "Base" value be the greatest in the table that was filled when the frame was encoded, you make it the greatest actually used by the frame. If that's not the newest entry, then it's not blocking, but the decoder doesn't care about that distinction, it just cares whether it has all referenced entries.

This does break your idea to use the HEADERS frames on the control stream as virtual checkpoints, though. The mid-instruction end of available data was also a challenge for @afrind's implementation, and that would be an issue again in this case.

@afrind
Copy link
Contributor

afrind commented Feb 22, 2018

@martinthomson : The current HTTP mapping spec sounds like you do send HEADER_ACK for table updates.

"The HEADER_ACK frame is sent on the Control Stream when the QCRAM decoder has fully processed a header block. It is used by the peer's QCRAM encoder to determine whether subsequent indexed representations that might reference that block are vulnerable to head-of-line blocking..."

I think this is correct. If you want to operate in fully blocking avoidance mode, the encoder needs to know the state of the decoder. Sounds like we need a separate issue to either clarify this or if someone wants to advocate for their removal.

Per this issue, I'm fine using magic to identify QCRAM streams. Note because you need to handle decrementing reference counts for outstanding header blocks on a stream that was reset, either QCRAM or some other layer needs to have a mapping from transport stream IDs to these blocks.

@martinthomson
Copy link
Member Author

@MikeBishop, if it weren't for the issue that @afrind points at, I'd say that we don't need HEADER_ACK. Right now I'd be inclined toward a different mechanism. That is, a decoder advertises its base periodically.

I don't have a great answer for blocking mid-instruction. Length prefixes might help, but they would also increase costs. I'm ambivalent on that from an implementation perspective. I wouldn't use them, but could easily ignore them and maybe validate them.

@afrind
Copy link
Contributor

afrind commented Feb 23, 2018

I like the property QMIN checkpoints introduced where the encoder decides when the decoder advertises synchronization, rather than the decoder advertising periodically. This aligns with our design principle of having the smarts in the encoder and the decoder is just following along. We could use HEADER blocks to signal these groups on the update stream. This is symmetric with request streams, which is a plus, but it doesn't provide a mechanism to leave a 'checkpoint' open for more than one request. If we feel like this is important we could borrow some of the checkpoint commands from QMIN (Insert Entry, Reuse Entry, Flush).

I think it's easier to write a decoder that doesn't have to deal with partial instructions and saving state, so I do prefer having some framing, but I may be selfish since that's how my decoder works and I just don't want to change it. We could possibly simulate it both ways and see how much wire overhead we're paying for code simplicity.

@MikeBishop
Copy link
Contributor

MikeBishop commented Feb 25, 2018

That should be relatively easy to instrument -- the wire overhead is the bytes used for an HTTP/QUIC frame header, on a somewhat-less-than-per-request frequency. A varint length, plus two fixed bytes, so relatively small. There's perhaps a latency cost in having them be on a shared stream with the other control data, but realistically this should be the bulk of it.

However, there's a difference from QMIN-style checkpoints that's important to remember: Checkpoints couldn't be used until the end of all checkpoint instructions (and QPACK allowed some flexibility here), while the granularity of consistent state here is on a per-header-entry basis. I think Martin's key insight here is that putting what is effectively a synchronization boundary here isn't required for consistency.

If we still want it for decoder simplicity, that's a choice we can make.

@afrind
Copy link
Contributor

afrind commented Mar 8, 2018

This issue has wandered a bit. I think the core of it -- moving compression messages off the HTTP control stream and replacing it with 4 unidirectional streams -- is fine.

There are separate issues open for framing on the control stream (#1122) and non-blocking decoder (#1140).

I generated some data about the overhead of framing on the control stream, but I'll put that in the corresponding issue.

@martinthomson martinthomson changed the title Separate QCRAM streams Separate QPACK streams Mar 15, 2018
afrind added a commit that referenced this issue Mar 19, 2018
1. Give QPACK encoders and decoders their own unidirectional streams (hardcoded to 6,7,10,11 for now)
2. Define simple framing for encoder stream
3. Move HEADER_ACK from HTTP over QUIC frame to QPACK instruction
4. Introduce Table State Synchronize instruction

Closes #1120 and #1121
@MikeBishop MikeBishop added the proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. label Mar 20, 2018
MikeBishop pushed a commit that referenced this issue Apr 24, 2018
1. Give QPACK encoders and decoders their own unidirectional streams (hardcoded to 6,7,10,11 for now)
2. Define simple framing for encoder stream
3. Move HEADER_ACK from HTTP over QUIC frame to QPACK instruction
4. Introduce Table State Synchronize instruction

Closes #1120 and #1121
afrind added a commit that referenced this issue Apr 26, 2018
1. Give QPACK encoders and decoders their own unidirectional streams (hardcoded to 6,7,10,11 for now)
2. Define simple framing for encoder stream
3. Move HEADER_ACK from HTTP over QUIC frame to QPACK instruction
4. Introduce Table State Synchronize instruction

Closes #1120 and #1121
afrind added a commit that referenced this issue May 15, 2018
1. Give QPACK encoders and decoders their own unidirectional streams (hardcoded to 6,7,10,11 for now)
2. Define simple framing for encoder stream
3. Move HEADER_ACK from HTTP over QUIC frame to QPACK instruction
4. Introduce Table State Synchronize instruction

Closes #1121 and #1122
afrind added a commit that referenced this issue May 15, 2018
* Move QPACK control instructions to dedicated streams

1. Give QPACK encoders and decoders their own unidirectional streams (hardcoded to 6,7,10,11 for now)
2. Define simple framing for encoder stream
3. Move HEADER_ACK from HTTP over QUIC frame to QPACK instruction
4. Introduce Table State Synchronize instruction

Closes #1121 and #1122

* +----+ => -----

* Fix Header Ack bit pattern, describe wire format using text
@mnot mnot removed proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. labels Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http -qpack design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

No branches or pull requests

5 participants