Subject: Re: [quicwg/base-drafts] Implicit acknowledgment via header block acknowledgment (#1399)

From: afrind <[email-masked]>
Date: Fri, 01 Jun 2018 13:50:39 -0700

afrind commented on this pull request.

> @@ -491,7 +491,14 @@ instruction begins with the '1' one-bit pattern. The instruction specifies the
 total number of dynamic table inserts and duplications since the last Table
 State Synchronize, encoded as a 7-bit prefix integer.  The encoder uses this
 value to determine which table entries are vulnerable to head-of-line blocking.
-A decoder MAY coalesce multiple synchronization updates into a single update.
+
+A decoder MAY coalesce multiple synchronization updates into a single update.  A
+decoder MAY rely on Header Acknowledgement instructions to indirectly
+acknowledge changes to the dynamic table.  Relying on Header Acknowledgment
+instructions exclusively leads to blocking if the encoder waits for an entry to

I don't think it would lead to blocking, but it would lead to never using the dynamic table?

> @@ -523,6 +530,12 @@ blocks within a stream have been fully processed.
 ~~~~~~~~~~
 {:#fig-header-ack title="Header Acknowledgement"}
 
+An encoder MUST treat receipt of a Header Acknowledgment as also acknowledging
+any dynamic table entries that the header block referenced.  That is, this
+instruction is also processed as a Table Size Synchronize instruction with a
+value matching the Largest Reference of the corresponding header block.

You mean TSS will be incremental from the largest acknowledged largest reference or most recent TSS?

> @@ -491,7 +491,14 @@ instruction begins with the '1' one-bit pattern. The instruction specifies the
 total number of dynamic table inserts and duplications since the last Table
 State Synchronize, encoded as a 7-bit prefix integer.  The encoder uses this
 value to determine which table entries are vulnerable to head-of-line blocking.
-A decoder MAY coalesce multiple synchronization updates into a single update.
+
+A decoder MAY coalesce multiple synchronization updates into a single update.  A
+decoder MAY rely on Header Acknowledgement instructions to indirectly
+acknowledge changes to the dynamic table.  Relying on Header Acknowledgment
+instructions exclusively leads to blocking if the encoder waits for an entry to
+be acknowledged before using it.  This happens if the encoder wants to avoid the
+risk of blocking at the decoder, or the decoder sets the
+SETTINGS_QPACK_BLOCKED_STREAMS lower than the number of active streams.

I had to read this many times to figure out what you are saying - you are explaining why the encoder might be waiting for acknowledgement before using the headers in the dynamic table.  I think it might be more clear if you change the second bit to something like

"... or the encoder has already reached the decoder's limit for blocked 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/1399#pullrequestreview-125305701

---

From: Martin Thomson <[email-masked]>
Date: Fri, 01 Jun 2018 16:10:32 -0700

martinthomson commented on this pull request.

 ~~~~~~~~~~

Yeah.  That way you maintain a single value for largest acknowledged entry and that is updated by TSS and header acknowledgment.  It's simpler and ensures correctness more directly.

-- 
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/1399#discussion_r192534928

---

From: Martin Thomson <[email-masked]>
Date: Fri, 01 Jun 2018 16:15:10 -0700

@martinthomson pushed 1 commit.

76be585  Make TSS incremental to Header Acknowledgment as well

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1399/files/68d8875981c1a02803dde70ae9038298e1ff7455..76be585ca759435bcc9e4e74d8e447176646449c

---

From: Mike Bishop <[email-masked]>
Date: Fri, 01 Jun 2018 16:16:09 -0700

MikeBishop commented on this pull request.

If the meaning of TSS is changing, then it's no longer a MAY for the decoder to let Header Acknowledgement instructions acknowledge changes.  The decoder MUST track the encoder's perspective of what has been acknowledged, and SHOULD send TSS in addition if entries have been added that aren't referenced.

-- 
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/1399#pullrequestreview-125340770

---

From: Mike Bishop <[email-masked]>
Date: Fri, 01 Jun 2018 16:18:45 -0700

I think this discussion no longer belongs inside the TSS instruction.  If the tracking of decoder state involves the encoder combining information from HA and TSS, and the decoder has to track the encoder's inferences from HA to generate TSS, this is a complex enough system to deserve its own subsection.

Then TSS simply says that it advances the greatest acknowledged index by however many entries, and references that subsection.

-- 
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/1399#issuecomment-394033454

---

From: Martin Thomson <[email-masked]>
Date: Fri, 01 Jun 2018 16:20:14 -0700

OK, that's not going to happen before I leave.  Let's continue next week.

-- 
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/1399#issuecomment-394033641

---

From: Martin Thomson <[email-masked]>
Date: Fri, 01 Jun 2018 16:21:08 -0700

74fa837  A start, but not an end

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1399/files/76be585ca759435bcc9e4e74d8e447176646449c..74fa837a81bea44b9f61a14c96342ee0c4c35985

---

From: afrind <[email-masked]>
Date: Wed, 13 Jun 2018 17:23:14 -0700

> @@ -523,6 +532,16 @@ blocks within a stream have been fully processed.
 ~~~~~~~~~~

Table *State* Synchronize.  It's wrong in the figure from that section also.

> @@ -489,9 +489,18 @@ After processing a set of instructions on the encoder stream, the decoder will
 emit a Table State Synchronize instruction on the decoder stream.  The
 instruction begins with the '1' one-bit pattern. The instruction specifies the
 total number of dynamic table inserts and duplications since the last Table
-State Synchronize, encoded as a 7-bit prefix integer.  The encoder uses this
-value to determine which table entries are vulnerable to head-of-line blocking.
-A decoder MAY coalesce multiple synchronization updates into a single update.
+State Synchronize or Header Acknowledgement that increased the largest

Reading the section here makes me think perhaps having Header Acknowledgement described before TSS.

-- 
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/1399#pullrequestreview-128607377

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Jun 2018 19:53:02 -0700

Yes, but I want to do that later.

-- 
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/1399#discussion_r195290416

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Jun 2018 19:54:02 -0700

386d628  State sync, not size sync

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1399/files/74fa837a81bea44b9f61a14c96342ee0c4c35985..386d6289be06efc70265e953f2bef3b095b14b65

---

From: afrind <[email-masked]>
Date: Fri, 15 Jun 2018 11:49:19 -0700

Closed #1399 via #1410.

-- 
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/1399#event-1684058128