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 correlation in Transport #672

Closed
wants to merge 16 commits into from
Closed

Conversation

MikeBishop
Copy link
Contributor

This is an augmentation of @martinthomson's PR to add stream correlation, as promised.

Major changes

Leveraging @igorlord's insight that OO=00 only occurs on the first STREAM frame of a stream, I used that as the trigger for a Stream Properties byte. Two bits of that byte describe the directionality of the stream:

  • Unidirectional (no response expected)
  • Initial bidirectional (one response expected)
  • Initial multi-response (one or more responses expected; needs a better name)
  • Response

If the type is Response, there's an Associated Stream ID field, length given by two more bits following the same pattern as the SS bits in the STREAM frame ID.

Personal Opinion

On the plus side, these stream types seem to cover the abstractions I can envision for most applications. You can unilaterally send something (unidirectional), do request/response (bidirectional), or pub/sub (single subscription stream, series of update streams).

I don't care for the fact that I still need the stream type header in HTTP after putting this in the transport. That will be ameliorated if we go back to one stream per request, since all unidirectional streams will be push streams. (As a side-note, I considered using the multiple-response option in the HTTP mapping, but then I need a stream header again to indicate which is the response and which the pushes.)

I particularly don't like that you now have to look at the frame type header to find out whether a field exists which tells you the length of something else in the header. I'd like to simplify that. I went with this model over a CREATE_STREAM frame because of @mikkelfj's use-case of very small messages -- this adds only one byte to the first frame on a stream in one direction and 2-5 bytes to the first frame of response streams. A separate frame type would be somewhat larger, but could be cleaner in that respect.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 28, 2017

I have so far only looked at transport.md - I think the exact encoding is not too important for now - notably var length integers might replace bits in the type byte.

One thought is that it might not be desirable to check for association errors because the initial stream might be closed and gone before a response arrives. Checking validity would require state to be retained - in principle indefinitely. But the association type may still be useful to an API layer.

@MikeBishop
Copy link
Contributor Author

MikeBishop commented Jun 28, 2017 via email

@martinthomson
Copy link
Member

@MikeBishop, do you think that #720 might be better and we could close this?

@MikeBishop
Copy link
Contributor Author

Closed in favor of #720.

@MikeBishop MikeBishop closed this Sep 14, 2017
@MikeBishop MikeBishop deleted the multidirectional branch September 23, 2017 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants