GOAWAY needs a maximum stream number in both directions

[martinthomson]

I have also taken the liberty of updating the text that surrounds the frame type.

@janaiyengar, @mnot, @larseggert, I believe that this is basically a bugfix and it doesn't need any more discussion that it has already. If you disagree, then we'll hold off on merging until we've had more discussion.

Closes #347.

[MikeBishop]

Generally looks good. I agree, this is bugfix-level.

[MikeBishop]

Nit: "even-numbered" and "odd-numbered" should be hyphenated.

[martinthomson]

From the commit comment:

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.

[mirjak]

Sorry if that's a little bit off topic but at this point I'm actually wondering if GOAWAY is really part of the transport semantics or should be an application layer signal...?

[martinthomson]

@mirjak, I can't actually think of a strong reason why GOAWAY has to be a transport concept. The ideas I came up with are:

1. It's nice to have the description of graceful shutdown in the transport document. On the other hand, the pattern is easy to establish and copy.

2. GOAWAY affects stream state. On higher numbered packets, you shouldn't see any mention of streams higher than the cutoff, and can generate errors accordingly. But those could be application-layer errors.

3. GOAWAY also removes the need to reset some streams. However, any streams that are implicitly reset by GOAWAY don't need to be reset before CONNECTION_CLOSE is sent. If those streams exist at the time that GOAWAY is sent, then there is no possible way to break the concurrent stream limit, which is the only other relevant transport-layer concern. There's a potential for local resource management optimizations when you send GOAWAY (discarding flow control windows etc...), but the cases where that applies are few - it only applies if you are cancelling streams that you know to be in progress, and that usually only happens when you are then sending an immediate CONNECTION_CLOSE.

In other words, yeah, we can make this an application-layer concern.

[MikeBishop]

I'm dubious that we'll land text on application-level clean shutdown before the draft deadline. Let's merge this bugfix for consistency in -02, then start on that change separately. If I'm wrong and we can close on it quickly, great!