Subject: Re: [quicwg/base-drafts] Clients can send GOAWAY too (#3129)

From: Mike Bishop <[email-masked]>
Date: Mon, 13 Jan 2020 13:39:00 -0800

Closed #3129.

-- 
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/3129#event-2944858993

---

From: Mike Bishop <[email-masked]>
Date: Mon, 13 Jan 2020 13:40:37 -0800

Didn't mean to close 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/3129#issuecomment-573884083

---

From: Mike Bishop <[email-masked]>
Date: Mon, 13 Jan 2020 13:40:39 -0800

Reopened #3129.

-- 
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/3129#event-2944863763

---

From: Mike Bishop <[email-masked]>
Date: Mon, 13 Jan 2020 13:41:27 -0800

@MikeBishop pushed 1 commit.

a6dcceaf92e90cda648c5f480289062de9976a43  Merge remote-tracking branch 'origin/master' into client-goaway

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/921f883ee450ce67ec7c94865ddd2809f6fbb579..a6dcceaf92e90cda648c5f480289062de9976a43

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 05:50:12 -0800

kazuho commented on this pull request.

> -stream is reset individually, or the connection terminates.  Servers MAY reject
-individual requests on streams below the indicated ID if these requests were not
-processed.
+connection and initiate a graceful connection close.  Endpoints initiate the
+graceful shutdown of a connection by sending a GOAWAY frame ({{frame-goaway}}).
+The GOAWAY frame contains an identifier that indicates to the receiver the range
+of requests or pushes that were or might be processed in this connection.  The
+server sends a client-initiated bidirectional Stream ID; the client sends a Push
+ID.  Requests or pushes with the indicated identifier or greater are rejected by
+the sender of the GOAWAY.  This identifier MAY be zero if no requests or pushes
+were processed.
+
+The information in the GOAWAY enables a client and server to agree on which
+requests or pushes were accepted prior to the connection shutdown.  Endpoints
+SHOULD abruptly terminate any requests or pushes that have identifiers greater
+than or equal to the smallest identifier sent in a GOAWAY frame.

I think we can remove this sentence, as we talk more specifically in the sentence starting from line 708 (i.e. "It is RECOMMENDED that the receiving endpoint explicitly reject such requests (see {{request-cancellation}}) or pushes (see {{frame-cancel-push}}) in order to clean up transport state for the affected streams."), and because the abstract is already given in the sentence starting from line 690 (i.e. "Requests or pushes with the indicated identifier or greater are rejected by the sender of the GOAWAY.").

-- 
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/3129#pullrequestreview-342537325

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 07:37:33 -0800

MikeBishop commented on this pull request.

This is talking about the sending endpoint, though.  I've received requests A-G; I'm sending a GOAWAY indicating that I've done some non-zero amount of processing up through C.  I send a GOAWAY indicating that D and higher haven't been processed, then SHOULD send RESET_STREAMs for D, E, F, and G to keep the transport state consistent.

-- 
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/3129#discussion_r366408943

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 10:51:54 -0800

> This is talking about the sending endpoint, though.

That's true, and I now notice that the sentence starting from line 708 is the one that's incorrect.

The paragraph starting from line 703 talks about what to do with messages that were already in transit. It is a generalization of an exiting paragraph, that states that the sender of GOAWAY SHOULD reset, the receiver of GOAWAY MAY. I assume that we want to retain those behaviors.

The second sentence of that paragraph that talks about the receiver is correct (i.e. "If the endpoint has already sent requests or pushes with an identifier greater than or equal to that received in a GOAWAY frame, those requests or pushes will not be processed"). Next sentence that starts from 708 should then talk about the sender, but it incorrectly states: _It is RECOMMENDED that the **receiving** endpoint explicitly reject such requests._

I think it is beneficial to retain a paragraph that concentrates on handling of the HTTP messages in transit, and therefore still think that removing this paragraph (starting from line 694) is the correct thing to do, _and also fixing the "receiving" in the sentence starting from line 708 to "sending."

-- 
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/3129#discussion_r366512233

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 13:07:02 -0800

I think the confusing piece here is that the sender of the GOAWAY is the receiver of the in-flight request/push.  This means to talk about the receipt of the request, but makes it sound like it's talking about the receiver of the GOAWAY.

I'm not sure we need to eliminate any paragraphs, but clarifying the terms would probably help a lot.  This section could previously rely on "client" and "server" to clearly imply who's doing what with requests and GOAWAY frames; now it's generalized and easy to lose track of the involved parties.

+The information in the GOAWAY enables a client and server to agree on which
+requests or pushes were accepted prior to the connection shutdown.  Endpoints
+SHOULD abruptly terminate any requests or pushes that have identifiers greater
+than or equal to the smallest identifier sent in a GOAWAY frame.
+
+Endpoints MUST NOT initiate new requests or pushes on the connection with an
+identifier greater than or equal to the smallest identifier received in a GOAWAY
+frame.  Clients MAY establish a new connection to send additional requests.
+
+Some requests or pushes might already be in transit. If the endpoint has already
+sent requests or pushes with an identifier greater than or equal to that
+received in a GOAWAY frame, those requests or pushes will not be processed;
+requests MAY be retried by the client on a different connection.  The endpoint
+that initiated these requests or pushes MAY cancel them.  It is RECOMMENDED that
+the receiving endpoint explicitly reject such requests (see

```suggestion
such requests be explicitly rejected (see
```

+requests or pushes were accepted prior to the connection shutdown.  Endpoints
+SHOULD abruptly terminate any requests or pushes that have identifiers greater
+than or equal to the smallest identifier sent in a GOAWAY frame.
+
+Endpoints MUST NOT initiate new requests or pushes on the connection with an
+identifier greater than or equal to the smallest identifier received in a GOAWAY
+frame.  Clients MAY establish a new connection to send additional requests.
+
+Some requests or pushes might already be in transit. If the endpoint has already
+sent requests or pushes with an identifier greater than or equal to that
+received in a GOAWAY frame, those requests or pushes will not be processed;
+requests MAY be retried by the client on a different connection.  The endpoint
+that initiated these requests or pushes MAY cancel them.  It is RECOMMENDED that
+the receiving endpoint explicitly reject such requests (see
+{{request-cancellation}}) or pushes (see {{frame-cancel-push}}) in order to

```suggestion
{{request-cancellation}} and {{frame-cancel-push}}) upon receipt in order to
```

-- 
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/3129#pullrequestreview-342835509

---

From: afrind <[email-masked]>
Date: Wed, 15 Jan 2020 14:00:23 -0800

@afrind pushed 5 commits.

510e0a9cc836b407a5e3517f4b9c032a6ce5e1f7  GOAWAY means no more, like H2
9d070060a798378f3f4ce1f5436f40e8a5381e91  Clarify the prohibition
c4568aa4366160b2046f84d5a29a443ec96f746a  Wordsmith
aa3f1c6d56abdbed3f7015ba46c656c63fb14864  Use 'GOAWAY frame' consistently
c712182e45511236dbb0a3d26c07817f201b2078  Clarify previously-promised

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

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:18:44 -0800

d6c141ffe639ef001ac28df6f86620288833d428  Merge remote-tracking branch 'origin/master' into client-goaway

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

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:21:28 -0800

f1c696608d527ea4f576db8d194db8e8d8790302  Apply my suggestions from code review

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

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:22:14 -0800

d0ef267257bbbfca76496dbbce137b7fb3271d5e  Apply my suggestions from code review

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

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:41:41 -0800

3f93d362a4573a5939f9e5e7740029debec24c5b  Clean up sender/receiver language

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/d0ef267257bbbfca76496dbbce137b7fb3271d5e..3f93d362a4573a5939f9e5e7740029debec24c5b

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:42:52 -0800

@kazuho, I've updated the text around both statements to try to make this clearer.  Can you take a look at 3f93d362?

-- 
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/3129#discussion_r369258453

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 13:48:01 -0800

37ee181ed7ad87d7d72d699b226a7e117edfc3a5  Adapt Martin's text from #3341

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/3f93d362a4573a5939f9e5e7740029debec24c5b..37ee181ed7ad87d7d72d699b226a7e117edfc3a5

---

From: Martin Thomson <[email-masked]>
Date: Tue, 21 Jan 2020 15:16:14 -0800

martinthomson commented on this pull request.

> @@ -1894,7 +1904,8 @@ PING (0x6):
 : PING frames do not exist, since QUIC provides equivalent functionality.
 
 GOAWAY (0x7):
-: GOAWAY is sent only from server to client and does not contain an error code.
+: GOAWAY does not contain an error code.  In the client to server direction,
+it carries a Push ID instead of a server initiated stream ID.

```suggestion
  it carries a Push ID instead of a server initiated stream ID.
```

-- 
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/3129#pullrequestreview-346257570

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jan 2020 15:19:38 -0800

59d84be618a99762f78539a09e2a579874ecc0ba  Update draft-ietf-quic-http.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/37ee181ed7ad87d7d72d699b226a7e117edfc3a5..59d84be618a99762f78539a09e2a579874ecc0ba

---

From: Martin Thomson <[email-masked]>
Date: Tue, 21 Jan 2020 17:39:01 -0800

> -
-Once all accepted requests have been processed, the server can permit the
-connection to become idle, or MAY initiate an immediate closure of the
-connection.  An endpoint that completes a graceful shutdown SHOULD use the
-H3_NO_ERROR code when closing the connection.
+when the server closes the connection.  An endpoint MAY send multiple GOAWAY
+frames indicating different identifiers, but MUST NOT increase the identifier
+value they send, since clients might already have retried unprocessed requests
+on another connection.
+
+An endpoint that is attempting to gracefully shut down a connection can send a
+GOAWAY frame with a value set to the maximum possible value (2^62-4 for servers,
+2^62-1 for clients). This ensures that the peer stops creating new requests or
+pushes. After allowing time for any in-flight requests or pushes to arrive, the
+endpoint can send another GOAWAY frame indicating which requests or pushes it
+will accept before the end of the connection. This ensures that a connection can

Let's not imply any certainty about this.

```suggestion
might accept before the end of the connection. This ensures that a connection can
```
warning: this might be > 80 columns now

-- 
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/3129#pullrequestreview-346302046

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jan 2020 09:01:33 -0800

94aaaef725600432a8826a590ac7d4f019fb190f  Update draft-ietf-quic-http.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/59d84be618a99762f78539a09e2a579874ecc0ba..94aaaef725600432a8826a590ac7d4f019fb190f

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jan 2020 09:03:25 -0800

fd1189fb0374992551201168b2a3beec6b5a76fd  Update draft-ietf-quic-http.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/94aaaef725600432a8826a590ac7d4f019fb190f..fd1189fb0374992551201168b2a3beec6b5a76fd

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jan 2020 20:09:53 -0800

kazuho 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/3129#pullrequestreview-347049089

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jan 2020 20:10:38 -0800

Thank you for all the work (including merging the changes from master). Looks good to me now.

-- 
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/3129#discussion_r369924026

---

From: Mike Bishop <[email-masked]>
Date: Tue, 28 Jan 2020 15:18:15 -0800

@martinthomson, you haven't approved and you had reservations before.  Is this PR in a state you can live with?

-- 
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/3129#issuecomment-579519507

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Jan 2020 01:01:12 -0800

martinthomson approved this pull request.

A few editorial suggestions, but I can live with this.

(I still think that signaling the push ID isn't entirely necessary, but I can see from this text how it might be used.  And I recognize that avoid asymmetry makes the marginal value of including the push ID easier to accept.)

> +The information in the GOAWAY frame enables a client and server to agree on
+which requests or pushes were accepted prior to the connection shutdown. Upon
+sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these

I think that we need to separate these here.  Requests are not processed, but pushes are merely not accepted.  There is a significant difference.

> +which requests or pushes were accepted prior to the connection shutdown. Upon
+sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these
+requests or pushes MAY cancel them.  Clients MAY retry such requests on a

I'm not sure that "MAY cancel" is the right thing to be saying here.  What you want to say is that these will be no longer serviced by the endpoint that initiates the shutdown, but the underlying transport resources (streams, etc...) will still exist, so disposing of those resources might be helpful.

> +sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these
+requests or pushes MAY cancel them.  Clients MAY retry such requests on a
+different connection.

I would make this a new paragraph rather than say "such requests".  This concept is important and this throwaway sentence obfuscates an important concept.  Consider moving this to the next paragraph.

> -for a client-initiated, bidirectional stream (i.e. 2^62-4 in case of QUIC
-version 1).  This GOAWAY frame signals to the client that shutdown is imminent
-and that initiating further requests is prohibited.  After allowing time for any
-in-flight requests to reach the server, the server can send another GOAWAY frame
-indicating which requests it will accept before the end of the connection. This
-ensures that a connection can be cleanly shut down without causing requests to
-fail.
-
-Once all accepted requests have been processed, the server can permit the
-connection to become idle, or MAY initiate an immediate closure of the
-connection.  An endpoint that completes a graceful shutdown SHOULD use the
-H3_NO_ERROR code when closing the connection.
+when the server closes the connection.  An endpoint MAY send multiple GOAWAY
+frames indicating different identifiers, but MUST NOT increase the identifier
+value they send, since clients might already have retried unprocessed requests
+on another connection.

This uses "endpoint", but this really only applies to clients.

-- 
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/3129#pullrequestreview-349942011

---

From: Mike Bishop <[email-masked]>
Date: Wed, 29 Jan 2020 10:09:12 -0800

I wound up separating these further, and in the process maybe breaking up the wall of text this section has become.

I reduced the MAY to a "can" and made it a separate paragraph further down.

By removing the intervening sentence, this seems like less of a throwaway sentence.  See if you like this flow better?

Symmetry.  If clients are going to send Push IDs, then I'd rather have the same processing rules in both directions -- even if the reason it really matters only applies to clients.

-- 
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/3129#discussion_r372543822

---

From: Mike Bishop <[email-masked]>
Date: Wed, 29 Jan 2020 10:09:45 -0800

04b43876bccfd669bd854ea7cc25704dae695729  Address @martinthomson's feedback

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3129/files/fd1189fb0374992551201168b2a3beec6b5a76fd..04b43876bccfd669bd854ea7cc25704dae695729