Remove error code and reason phrase from GOAWAY #355
Conversation
draft-ietf-quic-transport.md
Outdated
| beyond the error code. | ||
| Last Server Stream ID: | ||
|
|
||
| : The last server-initiated Stream ID which was accepted by the sender of the |
There was a problem hiding this comment.
"The last server-initiated Stream ID which was accepted by the sender of the GOAWAY frame" sounds a bit wonky.
There was a problem hiding this comment.
Yes, how about was created or will be created?
There was a problem hiding this comment.
Because the sender isn't necessarily the server, and the client isn't "creating" server-initiated streams. Maybe we could say "the highest ... ID on which the sender has processed or will process data"? But that doesn't seem more compact, so I'm not sure it's an improvement.
But regardless, that's feedback on #354, not this change in particular.
There was a problem hiding this comment.
I have taken this feedback in and will update #354 with this. I found something else too.
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.
martinthomson
force-pushed
the
goaway_for_no_reason
branch
from
14068b4
to
f167e31
Compare
draft-ietf-quic-transport.md
Outdated
|
|
||
| * Error Code: A 32-bit field error code which indicates the reason for closing | ||
| this connection. | ||
| Last Client Stream ID: |
There was a problem hiding this comment.
I might be biased towards Largest, instead of Last
draft-ietf-quic-transport.md
Outdated
| in the GOAWAY frame can then be retried. | ||
|
|
||
| For peer-initiated streams, an endpoint might indicate a lower value for the | ||
| highest stream number than the value that might be sent by a peer. After |
There was a problem hiding this comment.
I don't think sending a GOAWAY in response does anything unless the peer receiving the GOAWAY wants to further decrease one or both of the Last Stream IDs, so typically I wouldn't expect this is necessary. As such, I'd recommend changing this to MAY and explain it's only if the Last Stream IDs are too large.
There was a problem hiding this comment.
I've kept this as a SHOULD but added the requested conditions. It's important that "safe" retries are enabled as much as possible and failure to echo will create unnecessary uncertainty. This is a big deal in HTTP.
There was a problem hiding this comment.
You have to ack the GOAWAY, so why echo it, that would seem redundant.
There was a problem hiding this comment.
Actually, now that I think about it, the only reason why you would reduce the largest stream number is because you didn't use those streams, and if you don't want to use those streams, then you can just not use them. Let's remove this paragraph.
martinthomson
force-pushed
the
goaway_for_no_reason
branch
2 times, most recently
from
c8a3a23
to
000ad01
Compare
I have also taken the liberty of making the text that matches this more directly applicable.
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.
martinthomson
force-pushed
the
goaway_for_no_reason
branch
from
000ad01
to
6fcb7f9
Compare
In an error condition, CONNECTION_CLOSE is your friend.
Closes #352. Builds on #354.