Subject: [quicwg/base-drafts] GOAWAY needs a maximum stream number in both directions (#354)

From: Martin Thomson <[email-masked]>
Date: Wed, 01 Mar 2017 19:20:21 -0800

I have also taken the liberty of updating the text that surrounds the frame type.

@janaiyengar, @mnot, @larseggert, I believe that this is basically a bugfix and it doesn't need any more discussion that it has already.  If you disagree, then we'll hold off on merging until we've had more discussion.

Closes #347.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * GOAWAY needs a maximum stream number in both directions
  * Advice on use of GOAWAY during error handling

-- File Changes --

    M draft-ietf-quic-transport.md (66)

-- Patch Links --

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

---

From: Mike Bishop <[email-masked]>
Date: Thu, 02 Mar 2017 14:13:20 -0800

MikeBishop approved this pull request.

Generally looks good.  I agree, this is bugfix-level.

-* Reason Phrase Length: A 16-bit unsigned number specifying the length of the
-  reason phrase.  This may be zero if the sender chooses to not give details
-  beyond the error code.
+Last Client Stream ID:
+
+: The last client-initiated Stream ID which was accepted by the sender of the
+  GOAWAY frame.  All higher-numbered, client-initiated streams (that is, odd
+  numbered streams) are implicitly reset by sending or receiving the GOAWAY

Nit:  "even-numbered" and "odd-numbered" should be hyphenated.

-- 
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/354#pullrequestreview-24857568

---

From: Martin Thomson <[email-masked]>
Date: Thu, 02 Mar 2017 15:15:43 -0800

@martinthomson pushed 1 commit.

28b1821  Fix nits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/354/files/2f143edd123655db491780e52cbec1f0a680dc00..28b1821b5520a87716e1c029360d5ed00577054e

---

From: Martin Thomson <[email-masked]>
Date: Thu, 02 Mar 2017 15:40:20 -0800

5bf0bff  I've taken feedback from #355.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/354/files/28b1821b5520a87716e1c029360d5ed00577054e..5bf0bff25456c655c5264e7c3f6f55d4f0c322e7

---

From: Martin Thomson <[email-masked]>
Date: Thu, 02 Mar 2017 15:43:22 -0800

>From the commit comment:

I have also added text that captures something we learned late in the HTTP/2 experience.  GOAWAY pre-emptively closes streams, but for peer-initiated streams it might make sense to use a bigger number to allow any requests that the peer created in the last round trip to complete.  Some implementations find retrying quite difficult and allowing (for example) a client's last set of requests to complete can make the shutdown properly graceful (as opposed to toy-graceful).

The case where this matters is for a gateway to server, where the gateway can't reasonably retry the 10,000 requests that it initiated in the last second. To allow for that, the server can give the gateway a bit of extra leeway for those streams when it sends a GOAWAY frame.

-- 
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/354#issuecomment-283819270

---

From: mirjak <[email-masked]>
Date: Fri, 03 Mar 2017 04:08:36 -0800

Sorry if that's a little bit off topic but at this point I'm actually wondering if GOAWAY is really part of the transport semantics or should be an application layer signal...?

-- 
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/354#issuecomment-283937992

---

From: Martin Thomson <[email-masked]>
Date: Sun, 05 Mar 2017 14:14:48 -0800

@mirjak, I can't actually think of a strong reason why GOAWAY has to be a transport concept.  The ideas I came up with are:

1. It's nice to have the description of graceful shutdown in the transport document.  On the other hand, the pattern is easy to establish and copy.

2. GOAWAY affects stream state.  On higher numbered packets, you shouldn't see any mention of streams higher than the cutoff, and can generate errors accordingly.  But those could be application-layer errors.  

3. GOAWAY also removes the need to reset some streams.  However, any streams that are implicitly reset by GOAWAY don't need to be reset before CONNECTION_CLOSE is sent.  If those streams exist at the time that GOAWAY is sent, then there is no possible way to break the concurrent stream limit, which is the only other relevant transport-layer concern.  There's a potential for local resource management optimizations when you send GOAWAY (discarding flow control windows etc...), but the cases where that applies are few - it only applies if you are cancelling streams that you know to be in progress, and that usually only happens when you are then sending an immediate CONNECTION_CLOSE.

In other words, yeah, we can make this an application-layer concern.

-- 
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/354#issuecomment-284267581

---

From: Martin Thomson <[email-masked]>
Date: Sun, 05 Mar 2017 14:36:06 -0800

62fcb5f  s/Last/Largest/ and improve conditions on a triggered GOAWAY

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/354/files/5bf0bff25456c655c5264e7c3f6f55d4f0c322e7..62fcb5fde3c9ff0c8aeb5743ef658aa803abbb96

---

From: Martin Thomson <[email-masked]>
Date: Sun, 05 Mar 2017 14:57:06 -0800

4faa537  Remove GOAWAY echo

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/354/files/62fcb5fde3c9ff0c8aeb5743ef658aa803abbb96..4faa537e4d3f691a0642b92a6adc66d63c6287bb

---

From: Mike Bishop <[email-masked]>
Date: Wed, 08 Mar 2017 10:52:18 -0800

I'm dubious that we'll land text on application-level clean shutdown before the draft deadline.  Let's merge this bugfix for consistency in -02, then start on that change separately.  If I'm wrong and we can close on it quickly, great!

-- 
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/354#issuecomment-285132121

---

From: janaiyengar <[email-masked]>
Date: Wed, 08 Mar 2017 11:26:59 -0800

Merged #354.

-- 
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/354#event-991920747