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

Stream creation order #634

Closed
janaiyengar opened this issue Jun 15, 2017 · 20 comments
Closed

Stream creation order #634

janaiyengar opened this issue Jun 15, 2017 · 20 comments
Assignees
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@janaiyengar
Copy link
Contributor

Dmitri Tikhonov writes on the mailing list:

"Section 10.1 of draft-ietf-quic-transport-03 states that “Streams MUST be created in sequential order.” My question is: why mandate stream creation order, since the other side may receive out-of-order packets? Let the implementation do what it wants with Stream IDs – as long as it does not go over the limit, all should be well.

This phrase is a holdover from the previous version; this requirement seems to be unnecessary since issue #435 has been resolved."

I think this is right. Now that we have also replaced concurrent streams, this seems quite unnecessary. This will clean up some more text, but I wanted to air this issue here before turning it into a PR.

@janaiyengar janaiyengar added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Jun 15, 2017
@janaiyengar janaiyengar self-assigned this Jun 15, 2017
@martinthomson
Copy link
Member

In writing up the unidirectional stream proposal, I concluded that this was still a valuable restriction to have. Besides, creating streams out of order in that model isn't necessary.

@mnot mnot changed the title Streams need not be created in sequential order Stream creation order Jun 20, 2017
@dtikhonov
Copy link
Member

@martinthomson writes:

In writing up the unidirectional stream proposal, I concluded that this was still a valuable restriction to have.

Why?

@martinthomson
Copy link
Member

The same reason it is valuable now, but more so. The strict ordering restriction ensures that you can cheaply and reliably determine whether a stream is idle, open, or closed. Open streams need a bunch of state. If that state exists, the stream is open. Idle streams are those that don't have state and have higher numbers than all open streams. Closed streams are those that don't have state and have a lower number than any open stream.

Also, the only reason that you might want to open out of order is if you attached some sort of semantic to the stream ID. I think doing that is a mistake.

@mikkelfj
Copy link
Contributor

Could this "sequential" wording be made more precise? Is it OK to open stream 100, then 200, then 400, or should it be 100, 102, 104, ...

@mikkelfj
Copy link
Contributor

It matters because you have a period where you still accept stream frames before considering them an error - and if there are holes in the stream range it might confuse the logic determing this.

@dtikhonov
Copy link
Member

The strict ordering restriction ensures that you can cheaply and reliably determine whether a stream is idle, open, or closed.

I do not see how that is possible. Here is an example: server receives a request that opens stream 11, yet the largest peer-initiated stream it knows about is 7. Does it mean that a) client skipped stream 9 -- and thus is in violation of protocol -- or b) previous packet got lost?

Also, the only reason that you might want to open out of order is if you attached some sort of semantic to the stream ID.

Not necessarily. One can imagine a scenario in which a multi-threaded program keeps a pool of stream IDs that could be opened and distributes these IDs among threads. The threads perform some preliminary work -- streams are opened in the same order that these threads complete.

There are probably other reasons why an implementation may want to be able to control the order in which streams are created.

@ianswett
Copy link
Contributor

I would agree that this is still valuable. Unless there is an extremely compelling reason to remove the restriction(which I don't believe we have), we should keep it.

@RyanTheOptimist
Copy link
Contributor

RyanTheOptimist commented Sep 26, 2017 via email

@dtikhonov
Copy link
Member

@ianswett writes:

I would agree that this is still valuable.

Why?

@janaiyengar
Copy link
Contributor Author

I'm hearing the argument for keeping this restriction, which is that it allows for a simpler implementation. It doesn't break the protocol if the sender doesn't open streams sequentially. Together, these seem to suggest that we should keep this restriction, but make it a SHOULD from a MUST.

@mikkelfj
Copy link
Contributor

mikkelfj commented Sep 27, 2017 via email

@mikkelfj
Copy link
Contributor

mikkelfj commented Sep 27, 2017 via email

@dtikhonov
Copy link
Member

@mikkelfj writes:

It matters because you have a period where you still accept stream frames before considering them an error - and if there are holes in the stream range it might confuse the logic determing this.

Which stream frames do you mean? If stream frames for the streams that are in the "hole," then I do not see where the confusion would be: A stream frame for a stream that is not yet open opens that stream. If the stream count goes over the limit, a connection error is flagged and the connection is aborted.

@dtikhonov
Copy link
Member

@janaiyengar, @mikkelfj -- could you please elaborate on the simplification part?

@MikeBishop
Copy link
Contributor

MikeBishop commented Sep 27, 2017

I think I can partially explain it. Each stream for which you're tracking state consumes some memory. You can elide out large chunks by saying that:

  • Lowest idle stream ID is X
  • Of streams < X, any stream not in my state table is closed
  • Table contains any idle or open streams < X

If an implementation is implementing tracking in this way, the arrival of stream 200 when X=100 means inserting 100,102,...,198 into the table as idle, then 200 as open. By mandating that they be used in order, even if reordering occurs, you are less likely to do large chunks of this work at a time (you might get 108 when X=100, but not 200) and if you do have to do a large chunk of work the work is still timely because you know all those streams have data in-flight so you would have been doing the work in a few packets anyway.

@dtikhonov
Copy link
Member

@MikeBishop, I see -- thank you for the explanation. @martinthomson must have been also alluding to this
when he wrote that "open streams need a bunch of state." The draft says that

opening a stream causes all lower-numbered streams in the same direction to become open.

Now I disagree with arguments for mandating stream creation order on two counts -- implementation and protocol specification.

I. Implementation.

The way to track open streams that you describe is suboptimal. The assertion that "open streams need a bunch of state" is not true: this is because stream transition from idle to open states can be treated in a purely conceptual fashion. Real stream state -- that is, some data structures and linkages -- does not have to be allocated until the first frame received for that stream. This is how we chose to implement it at LiteSpeed, guided by a sort of rule of thumb "delay doing work until you have to."

As a side note, doing a large chunk of work to allocate stream resources is not always timely: consider the case when a connection is closed shortly after this chunk of work is performed. This is more likely to occur than not in this situation, as a large hole in the sequence of incoming stream IDs may be a symptom of network issues.

II. Protocol Specification

A protocol should strive to be simpler rather than more complex. If something is not required by the protocol itself -- which the order of stream creation is not -- it does not belong in the specification. The protocol specification should not be a place where the way to implement something is mandated. This may prevent a clever implementer from employing some optimization mechanism that the protocol authors had not thought about.

@martinthomson
Copy link
Member

Your implementation point there seems to motivate the opposite. As Mike said, receiving stream X might open stream X-2, but the receiver doesn't have to do anything at that point.

Here's a different way to think about this: why do you want to open streams out of order?

@dtikhonov
Copy link
Member

@martinthomson, my point was that an efficient stream-tracking logic is not difficult achieve even in the face of out-of-order stream creation. Let the implementer figure out what to do, not mandate how to do it.

I do not want to create streams out of order -- HTTP over QUIC does not require it. Nevertheless, I think that the scenario I described above is plausible -- why put in constraints that would make in infeasible?

@mikkelfj
Copy link
Contributor

mikkelfj commented Sep 28, 2017 via email

@ThomasSwindells
Copy link

ThomasSwindells commented Sep 28, 2017 via email

martinthomson added a commit that referenced this issue Oct 20, 2017
This takes the state machines that @MikeBishop showed at the Seattle interim
and turns them into text and ASCII art.

I made a few changes in the process.  The state where all data is available
didn't turn out to be any different to the state preceding it, so I removed it.
I also found it valuable to recognize that streams might be created for sending
before any frames are sent (I considered doing the same for receive streams,
but it complicated the transitions more than I liked, I could be convinced to
add this still, it just seems less valuable).

I added a section on composite states, showing how this mess all maps to stuff
you already know (h2).

This is still work-in-progress.  I have changed the main chunk of text on
states, but I haven't changed any of the rest of the document, which still
talks about "idle" and all that.  I'll do that next if people are generally
happy with the direction this is headed.

Closes #634.
martinthomson added a commit that referenced this issue Nov 30, 2017
This takes the state machines that @MikeBishop showed at the Seattle interim
and turns them into text and ASCII art.

I made a few changes in the process.  The state where all data is available
didn't turn out to be any different to the state preceding it, so I removed it.
I also found it valuable to recognize that streams might be created for sending
before any frames are sent (I considered doing the same for receive streams,
but it complicated the transitions more than I liked, I could be convinced to
add this still, it just seems less valuable).

I added a section on composite states, showing how this mess all maps to stuff
you already know (h2).

This is still work-in-progress.  I have changed the main chunk of text on
states, but I haven't changed any of the rest of the document, which still
talks about "idle" and all that.  I'll do that next if people are generally
happy with the direction this is headed.

Closes #634.
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

9 participants