Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does GOAWAY really need a stream number? #1853

Closed
MikeBishop opened this issue Oct 11, 2018 · 14 comments
Closed

Does GOAWAY really need a stream number? #1853

MikeBishop opened this issue Oct 11, 2018 · 14 comments
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.

Comments

@MikeBishop
Copy link
Contributor

MikeBishop commented Oct 11, 2018

In HTTP/2, GOAWAY caused actual stream state transitions on streams higher than the indicated ID. Since QUIC handles stream state, this can't occur automatically in HTTP/QUIC. Instead,

Once [GOAWAY is] sent, the server MUST cancel requests sent on streams with an identifier higher than the indicated last Stream ID. Clients MUST NOT send new requests on the connection after receiving GOAWAY.... If the client has sent requests on streams with a higher Stream ID... those requests are considered cancelled. Clients SHOULD reset any streams above this ID with the error code HTTP_REQUEST_CANCELLED.

So the server effectively cancels on the client twice, once using HTTP and once using QUIC. Clients SHOULD reset some of the requests (on the expectation that the server will anyway), but the server will probably also do it for them. This feels a little broken.

If the clients can't send new requests after receiving the GOAWAY and the streams are going to get RST_STREAM/STOP_SENDING from one or both parties anyway, the stream ID in the GOAWAY frame seems like it provides marginal value.

Basically the only effect is that clients can anticipate that requests issued while the GOAWAY was in flight (i.e. greater than the identified stream ID) will likely be cancelled and be valid for retry. They can't technically be sure unless that happens, but might choose to retry requests sooner anyway.

It would be more correct to wait for the cancellation to arrive to know that's the error code the server chose. Perhaps the right model would be:

  • GOAWAY contains no payload. Clients MUST NOT initiate new requests after receiving GOAWAY, but it conveys no information about the state of the requests already in flight.
  • Clients MAY either cancel partially sent requests or finish sending them and let the server decide.
  • After sending a GOAWAY, servers MUST promptly cancel any request from the client which it does not intend to fulfill.
  • Once all requests have completed (either via a response or cancellation), the client either sends an APPLICATION_CLOSE NO_ERROR or allows the connection to terminate via the idle timeout.
@MikeBishop MikeBishop added design An issue that affects the design of the protocol; resolution requires consensus. -http labels Oct 11, 2018
@kazuho
Copy link
Member

kazuho commented Oct 11, 2018

I think that this is a good suggestion and I support making this change.

In QUIC, there is no guarantee that the server will receive the HTTP requests in ascending order of stream IDs (since we have out-of-order delivery). Having different behavior based on a particular stream ID carried by a GOAWAY frame does not make sense to me.

@MikeBishop
Copy link
Contributor Author

Continuing to read through the closure section, it calls out one scenario which might be a partial justification:

  • POST request is in flight from client
  • QUIC *_CLOSE frame is received from server

Can the POST be retried? If the packet also included a STREAM frame containing a GOAWAY, the client can perhaps conclude that the POST was not in the potentially-processed range. More generally, it provides a modicum of guidance on what to do with open streams if the connection closes with streams still active. However, this actually happening seems somewhat remote and I'm dubious that it justifies retaining a two-layer close mechanism.

@kazuho
Copy link
Member

kazuho commented Oct 11, 2018

Regarding *_CLOSE, can we say that the server SHOULD use GOAWAY to initiate close?

@MikeBishop
Copy link
Contributor Author

We already advise that, if the server has advance notice that a _CLOSE will be necessary.

@afrind
Copy link
Contributor

afrind commented Oct 11, 2018

It seems a bit silly to have the client wait for the server cancellation of the request to decide if it can be retried, when the GOAWAY could have contained that information. Suppose the client has sent a request that has not been fully acknowledged, and it receives a GOAWAY which would have told it the request was not received. Now it may have to wait to retransmit part the request just so the server can cancel it?

@kazuho
Copy link
Member

kazuho commented Oct 12, 2018

@MikeBishop

We already advise that, if the server has advance notice that a _CLOSE will be necessary.

Thank you. I now see that it is stated in section 5.2.

@afrind

It seems a bit silly to have the client wait for the server cancellation of the request to decide if it can be retried, when the GOAWAY could have contained that information.

That's true, but I might argue that we have such case already. Due to packet loss, a server that sends GOAWAY with stream ID X might not have seen stream X - 4. In such case, a client needs to retransmit stream X - 4 until it receives cancellation from server.

My understanding is that in the most typical case a server will send GOAWAY to stop the client initiating new streams, then close the connection when all the requests have been served. In such case, stream ID field in GOAWAY is useless. The issue is about a case where servers want to terminate a connection somewhat urgently but not urgent enough to send APPLICATION_CLOSE, which is IMO would be somewhat rare, and I do not mind having some inefficiency for the sake of simplicity here.

@martinthomson
Copy link
Member

I'm with @afrind here, the purpose of the GOAWAY is to tell the client which requests are safe to retry efficiently.

Once you have that in place the rest is bookkeeping. If you want to continue to send requests on stream X and above, that's something the transport will let you do, but you still won't get a response (and you can still try them safely on another connection).

@ianswett
Copy link
Contributor

I agree with @afrind, but I also think this largely invalidates any effort to not expose stream IDs from the transport layer to the application(ie: HTTP) layer?

I'm personally fine with exposing these IDs, since it's zero-cost in implementation(the application needs some identifier), but I believe others have been trying to remove them? A consistent approach here would be helpful, since exposing them and acknowledging that's ok/necessary could simplify applications.

@MikeBishop
Copy link
Contributor Author

The line I've been trying to hold is that there shouldn't be a situation in which the application goes to the transport and says "Give me stream 7." It says "Give me a stream," then examines the result and discovers it got 7. Using them as identifiers once they exist is fine, but they're identifiers issued by and under the control of the transport.

Similarly, I've been trying not to bake assumptions about the relationship between ID and type into the application layer; the ID and the type are independent properties; any relationship between them is the business of the transport.

@ianswett
Copy link
Contributor

That seems like a good line, and in practice, it's about what our current implementation does. Thanks for clarifying Mike.

@LPardue
Copy link
Member

LPardue commented Oct 16, 2018 via email

@ddragana
Copy link
Contributor

The line I've been trying to hold is that there shouldn't be a situation in which the application goes to the transport and says "Give me stream 7." It says "Give me a stream," then examines the result and discovers it got 7. Using them as identifiers once they exist is fine, but they're identifiers issued by and under the control of the transport.

There is one corner case(it is really a corner case): Early-data is used on a connection and the application opens a couple of streams(let's say n). Its cancel one of the streams(let's say stream k) before getting early-data-not-accepted. so we do not need to resend the canceled stream after connection is established. If we do not resend the stream we need to change streamID for streams with streamID k+1..n. The streamID of a stream can change in lift time of a stream.

@MikeBishop
Copy link
Contributor Author

Early data resets everything about the connection, including the application state. The connection starts over again from stream zero and has no obligation to contain the same data on the same streams, or even the same requests.

When 0-RTT is rejected, all connection characteristics that the client assumed might be incorrect. This includes the choice of application protocol, transport parameters, and any application configuration. The client therefore MUST reset the state of all streams, including application state bound to those streams.

@MikeBishop
Copy link
Contributor Author

I don't think the case is strong enough here to warrant a change. I'll try to improve the text instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

No branches or pull requests

7 participants