Subject: [quicwg/base-drafts] Clarify QPACK eviction races (#1237)

From: afrind <[email-masked]>
Date: Mon, 19 Mar 2018 06:26:04 -0700

Fixes #1129
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Clarify QPACK eviction races

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Mar 2018 14:07:46 +0000 (UTC)

martinthomson commented on this pull request.

>  desired.
 
-To ensure that the blocked eviction case is rare, references to the oldest
-entries in the dynamic table SHOULD be avoided.  When one of the oldest entries
-in the table is still actively used for references, the encoder SHOULD emit an
-Indexed-Duplicate representation instead (see {{indexed-duplicate}}).
+To ensure that blocked evictions are rare, the encoder SHOULD avoid
+referencing any eviction-prone entries, which are dynamic table entries that may
+be evicted soon.  Rather than reference an eviction-prone entry, the encoder
+SHOULD emit an Indexed-Duplicate representation (see {{indexed-duplicate}}),
+and reference the duplicate instead.
+
+To identify eviction-prone entries, the encoder may maintain a draining index,
+which is the smallest index in the dynamic table that it will emit a
+reference for.  As new entries are inserted, the encoder increments the
+draining index such that the amount of free and draining space in the dyanmic
+table is larger than its target threshold.

I'm fairly stupid during meetings that I pay attention to, but I didn't follow this last paragraph very well.  Smallest implies small numbers, but I think that you mean smallest "base" (which is the inverse of indices).

I probably don't really understand what you are trying to say, so consider tweaking for clarity. That might mean more text.

-- 
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/1237#pullrequestreview-104974911

---

From: afrind <[email-masked]>
Date: Mon, 19 Mar 2018 15:54:03 -0700

afrind commented on this pull request.

Sorry, I meant the smallest absolute index.  I wouldn't call it a base, since I think of a base as an absolute index that you encode against.  I'm guessing that alone isn't enough to clarify?

I think of draining space as space in the table occupied by valid entries that the encoder chooses not to use.

Here's the little snippet of code that I use to implement, where `minUsable_` represents my draining index:

```
  while (capacity_ - bytes_ + drainedBytes_ < table_->minFree() &&
         minUsable_ <= baseIndex_) {
    VLOG(5) << "Draining absolute index " << minUsable_;
    drainedBytes_ += (*table_)[absoluteToInternal(minUsable_++)].bytes();

  }
```

-- 
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/1237#discussion_r175610917

---

From: Mike Bishop <[email-masked]>
Date: Tue, 20 Mar 2018 10:43:26 +0000 (UTC)

MikeBishop approved this pull request.

-- 
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/1237#pullrequestreview-105294301