Subject: [quicwg/base-drafts] GOAWAY initiating a graceful shutdown can carry a huge stream ID (#3343)

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 15:39:02 -0800

Clarify that when initiating a graceful shutdown, sending a stream ID that is greater than the current maximum communicated to the peer is as correct as sending the current maximum.

This is because we already state that _clients MUST NOT send new requests on the connection after receiving GOAWAY; a new connection MAY be established to send additional requests_ ([section 5.2](https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-5.2-3)).

As discussed in #3341, such change helps us minimize the API exposed by the QUIC transport.

In addition, the proposed text talks refers to the stream concurrency of QUIC as a concept, rather than talking about a specific frame (i.e. MAX_STREAMS).

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

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

-- Commit Summary --

  * GOAWAY initiating a graceful shutdown can carry a huge stream ID / also talk about QUIC concepts

-- File Changes --

    M draft-ietf-quic-http.md (14)

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 14 Jan 2020 16:01:59 -0800

ianswett commented on this pull request.

LG

> @@ -727,12 +727,14 @@ indicating different stream IDs, but MUST NOT increase the value they send in
 the last Stream ID, since clients might already have retried unprocessed
 requests on another connection.  A server that is attempting to gracefully shut
 down a connection SHOULD send an initial GOAWAY frame with the last Stream ID
-set to the maximum value allowed by QUIC's MAX_STREAMS and SHOULD NOT increase
-the MAX_STREAMS limit thereafter.  This signals to the client that a shutdown is
-imminent and that initiating further requests is prohibited.  After allowing
-time for any in-flight requests (at least one round-trip time), the server MAY
-send another GOAWAY frame with an updated last Stream ID.  This ensures that a
-connection can be cleanly shut down without losing requests.
+set to the maximum value allowed by the concurrency control of QUIC for the
+client-initiated, bidirectional streams (see section 4.5 of {{QUIC-TRANSPORT}})
+or any value above that, and SHOULD forbid the QUIC transport from granting
+additional concurrency credits thereafter.  This signals to the client that a
+shutdown is imminent and that initiating further requests is prohibited.  After
+allowing time for any in-flight requests (at least one round-trip time), the
+server MAY send another GOAWAY frame with an updated last Stream ID.  This
+ensures that a connection can be cleanly shut down without losing requests.

```suggestion
ensures that a connection can be cleanly shut down without causing requests to fail.
```

-- 
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/3343#pullrequestreview-342916931

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 Jan 2020 16:06:21 -0800

LPardue commented on this pull request.

I like the overall direction of the PR. However, the text suggesting at least one round-trip time seems like an API to QUIC that is over exposing. AFAICT, this is the only such text that mentions a H3 behaviour in relation to RTT.

-- 
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/3343#pullrequestreview-342919079

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 16:23:04 -0800

kazuho commented on this pull request.

This is existing text, therefore I think we deserve a separate issue if we want to change this.

-- 
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/3343#discussion_r366641638

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 Jan 2020 16:27:29 -0800

Agreed, let me open one.

-- 
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/3343#discussion_r366642725

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 16:34:59 -0800

@kazuho pushed 1 commit.

306fba5aa1ba0200979249a96fbacfa5b3d9855b  apply @ianswett's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3343/files/38802a847ea7ca407b0bafde41392fc3cd3d8cb9..306fba5aa1ba0200979249a96fbacfa5b3d9855b

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 16:35:25 -0800

Thank you for the suggestion. Applied in 306fba5.

-- 
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/3343#discussion_r366644762

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 Jan 2020 16:36:08 -0800

LPardue 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/3343#pullrequestreview-342928116

---

From: afrind <[email-masked]>
Date: Tue, 14 Jan 2020 17:26:58 -0800

afrind commented on this pull request.

> @@ -727,12 +727,15 @@ indicating different stream IDs, but MUST NOT increase the value they send in

This seems to reference an API between the HTTP and QUIC transport layers?  Would it be better to just say the server SHOULD NOT grant any more concurrency credit?

-- 
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/3343#pullrequestreview-342942674

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 17:57:52 -0800

@kazuho pushed 1 commit.

a270887fcc6b8b8dd1f0dad780d5551e779a27cd  do not assume that there's a API between QUIC and H3

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3343/files/306fba5aa1ba0200979249a96fbacfa5b3d9855b..a270887fcc6b8b8dd1f0dad780d5551e779a27cd

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 17:58:20 -0800

That's a good point. I've updated the text in a270887, please take a look.

-- 
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/3343#discussion_r366663805

---

From: Mike Bishop <[email-masked]>
Date: Wed, 15 Jan 2020 08:17:43 -0800

MikeBishop approved this pull request.

Looks both beneficial and editorial.

-- 
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/3343#pullrequestreview-343342574

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 15 Jan 2020 14:30:27 -0800

janaiyengar approved this pull request.

```suggestion
set to the maximum value allowed by the concurrency control of QUIC for
```

-- 
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/3343#pullrequestreview-343566168

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jan 2020 15:09:56 -0800

martinthomson commented on this pull request.

Thanks for doing this Kazuho.  I do have some questions though.  I realize that the original was unclear and I suspect that this is now more unclear.

```suggestion
client-initiated, bidirectional streams (see Section 4.5 of {{QUIC-TRANSPORT}})
```

+or any value above that, and SHOULD NOT grant any more concurrency credit at the

I would say "could request that the transport not grant any more ..." as this is does not need to carry normative force.  But that's not text you changed, so I'll leave that for @MikeBishop to sort out.

Is this 2^62-4 or the current maximum stream limit (for c-i, bidi)?

Concretely, I think that if we are making a recommendation, we should permit either.  The difference between those choices should be clear enough to implementers once we lay it out like that.

-- 
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/3343#pullrequestreview-343581669

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 16:56:25 -0800

@kazuho pushed 1 commit.

f39bcc42a4064d23c2f8940a806477d54ce7cc49  capitalize "Section"

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3343/files/a270887fcc6b8b8dd1f0dad780d5551e779a27cd..f39bcc42a4064d23c2f8940a806477d54ce7cc49

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 16:56:47 -0800

Thanks! Done in f39bcc4.

-- 
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/3343#discussion_r367183658

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 16:57:31 -0800

:+1: FWIW, I agree to what @martinthomson states.

-- 
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/3343#discussion_r367183847

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 18:19:35 -0800

@kazuho pushed 2 commits.

1d54261e83519d930cce7224cbd0691cf465e2bd  "currently" allowed max stream
c4a513c577cff4603e803364f0bf3165b3bd9bb9  rearrange for better reading

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3343/files/f39bcc42a4064d23c2f8940a806477d54ce7cc49..c4a513c577cff4603e803364f0bf3165b3bd9bb9

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 18:22:28 -0800

Yes. This PR is (or have been) about agreeing both. I've added "currently"  at 1d54261, and made editorial adjustments in c4a513c.

-- 
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/3343#discussion_r367202602

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 18:30:23 -0800

FWIW, it is my understanding that the last non-editorial change regarding GOAWAY was #1712, which had "currently". It seems that we have dropped that while making editorial changes.

-- 
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/3343#discussion_r367204256

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 18:50:29 -0800

PS. I think there is a bigger issue here, see https://github.com/quicwg/base-drafts/issues/3341#issuecomment-574956848.

-- 
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/3343#discussion_r367208145

---

From: Martin Thomson <[email-masked]>
Date: Thu, 16 Jan 2020 03:03:12 +0000 (UTC)

martinthomson approved this pull request.

Thanks for taking my comments into consideration.

-- 
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/3343#pullrequestreview-343650000

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 19:37:07 -0800

@kazuho pushed 1 commit.

09382208a0942fd6281102a2f6c2e4ed279ecd60  stop talking about adjusting transport behavior, nor use RFC2119 keywords

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3343/files/c4a513c577cff4603e803364f0bf3165b3bd9bb9..09382208a0942fd6281102a2f6c2e4ed279ecd60

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jan 2020 19:39:29 -0800

I've applied what @martinthomson and I have discussed in #3341 (up to [this comment](https://github.com/quicwg/base-drafts/issues/3341#issuecomment-574964216)).

-- 
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/3343#issuecomment-574967511

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Jan 2020 12:23:16 -0800

Merged #3343 into master.

-- 
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/3343#event-2959390250