Subject: [quicwg/base-drafts] Discuss HTTP/QUIC Connection Closure (#1712)

From: Mike Bishop <[email-masked]>
Date: Tue, 28 Aug 2018 12:10:54 -0700

Fixes #1599.  Note that the MAX_PUSH_ID section is a red herring -- I moved so much text from the GOAWAY frame definition to this new section that GH thinks I moved the text in between instead.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Connection closure section
  * Re-add GOAWAY text

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 28 Aug 2018 15:46:16 -0700

martinthomson commented on this pull request.

The restructuring was a little distracting, but this looks like a good change overall. I have a few suggestions.  I didn't scrub carefully for grammar, but you are better at that than I anyway :)

> +do so if approaching the idle timeout.
+
+HTTP clients are expected to use QUIC PING frames to keep connections open while
+there are responses outstanding for requests or server pushes. If the client is
+not expecting a response from the server, allowing an idle connection to time
+out is preferred over expending effort maintaining a connection that might not
+be needed.  A gateway MAY use PING to maintain connections in anticipation of
+need rather than incur the latency cost of connection establishment to servers.
+Servers SHOULD NOT use PING frames to keep a connection open.
+
+## Connection Shutdown
+
+Even when a connection is not idle, either endpoint can decide to stop using the
+connection.  Clients do this by not sending additional requests on the
+connection; responses and pushed responses associated to previous requests will
+continue to completion.

This introduction doesn't mention that the intent is to be "graceful" or to close the connection without losing requests.  That's important, because it is also possible to just slam the connection closed.

> @@ -1102,69 +1167,33 @@ 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.
 
-Once all requests on streams at or below the identified stream number have been
-completed or cancelled, and all promised server push responses associated with
-those requests have been completed or cancelled, the connection can be closed
-using an Immediate Close (see {{QUIC-TRANSPORT}}).  An endpoint that completes a
-graceful shutdown SHOULD use the QUIC APPLICATION_CLOSE frame with the
-HTTP_NO_ERROR code.
+Once all accepted requests have been processed, the server MAY permit the

I think s/MAY/can here.  The normative language elsewhere covers this.

+## Explicit Application Closure

Explicit or immediate?  Better to use the same terminology as the transport doc, methinks.  Explicit, taken to mean "direct", might be right, but it seems a little off.

-# Connection Management
+Before closing the connection, a GOAWAY MAY be sent to allow the client to retry
+requests.  Including the GOAWAY frame in the same packet as the QUIC

"retry some requests" - the implication here is that GOAWAY is sent immediately before, which would limit its usefulness.

+If a connection terminates without a GOAWAY frame, clients MUST assume
+that any request which was sent might have been processed.

Maybe mention "in whole or part" when it comes to requests, even a partially sent request can be processed.

-- 
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/1712#pullrequestreview-150344894

---

From: Luca Niccolini <[email-masked]>
Date: Tue, 28 Aug 2018 17:44:09 -0700

lnicco commented on this pull request.

> +the error code HTTP_REQUEST_CANCELLED.  Servers MAY also cancel requests on
+streams below the indicated ID if these requests were not processed.
+
+Requests on Stream IDs less than or equal to the Stream ID in the GOAWAY frame
+might have been processed; their status cannot be known until they are completed
+successfully, reset individually, or the connection terminates.
+
+Servers SHOULD send a GOAWAY frame when the closing of a connection is known
+in advance, even if the advance notice is small, so that the remote peer can
+know whether a stream has been partially processed or not.  For example, if an
+HTTP client sends a POST at the same time that a server closes a QUIC
+connection, the client cannot know if the server started to process that POST
+request if the server does not send a GOAWAY frame to indicate what streams it
+might have acted on.
+
+An endpoint MAY send multiple GOAWAY frames if circumstances change. For

I know this commit is just moving text around, but I wanted to note how the beginning of this paragraph seems to be for the most part the same as in HTTP2 and does not take into consideration that in HQ there is no error code in the GOAWAY frame. So perhaps the "sends GOAWAY without an error code" can be removed ?

In HQ the only reason I can think of for the endpoint to send multiple GOAWAY frames is to first signal the intention to drain the connection, so that the remote endpoint on receipt stops creating new streams, and then after 1RTT (or on confirmation that the GOAWAY was received) the local endpoint can actually stop accepting new streams and seal the last stream ID and send a new GOAWAY frame. The remote endpoint upon receipt can now reset streams it thinks are established but in truth are not, with HTTP_REQUEST_CANCELLED, as explained above. 

-- 
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/1712#pullrequestreview-150367939