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

Implicitly acknowledging table updates by acknowleding header blocks #1370

Closed
martinthomson opened this issue May 22, 2018 · 6 comments
Closed
Labels
-qpack design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@martinthomson
Copy link
Member

When you acknowledge a header block, you also implicitly acknowledge the largest reference used in that header block. You can't acknowledge it otherwise.

Should we require that an encoder track this information so that a decoder can reduce the number of acknowledgments that it sends? It's more complicated to do that, but it can save some bytes and the associated churn. I haven't implemented delayed acknowledgments for QPACK, but this would possibly make that more effective.

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -qpack labels May 22, 2018
@afrind
Copy link
Contributor

afrind commented May 22, 2018

The encoder must already know the set of headers referenced in a block, so it can trivially compute the largest reference without keeping any additional state. I think we can say that acknowledging a header block implicitly acknowledges its largest reference and save the bytes on the wire.

@MikeBishop
Copy link
Contributor

Given that the entries are usually added to the table precisely because a header block is going to use them, this would probably cover acknowledgements most of the time. I'd be okay with that; you're already having to track which entries the block referenced, so this really isn't much additional state, if any. Just an additional action with that state when you pull a block out of the queue.

@martinthomson
Copy link
Member Author

It sounds like this is OK. However, I would caution that if you are avoiding blocking for new insertions, then this could deadlock. Decoders need to be aware of that.

@afrind
Copy link
Contributor

afrind commented May 25, 2018

@martinthomson : are you saying that a decoder can't rely exclusively on this ack mechanism or risk deadlock -- eg: if largest reference is less than base index the decoder should be sure to ack the remainder. Or something else?

@martinthomson
Copy link
Member Author

My point was that unless we require encoders to understand this "implicit" or indirect acknowledgement, then we risk them getting into a situation where the decoder thinks that it acknowledged something, but the encoder still believes it to be outstanding.

@afrind
Copy link
Contributor

afrind commented May 28, 2018

Yes, if we allow decoders to do this then it's a MUST for encoders to support it.

@afrind afrind closed this as completed in 031f07f Jun 15, 2018
igorlord pushed a commit to igorlord/quic-base-drafts that referenced this issue Jun 18, 2018
* Implicit acknowledgment via header block acknowledgment

Closes quicwg#1370.

* Make TSS incremental to Header Acknowledgment as well

* A start, but not an end

* State sync, not size sync

* Acknowledge stream resets too

Closes quicwg#1371.

* Fix figure

* Genericize

* Plural, not plural

* Editorial

* Itsan instruction

* QPACK Feedback with forward references

* Alan's text
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-qpack design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

4 participants