-
Notifications
You must be signed in to change notification settings - Fork 23
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
What happens when FETCH_HEADER is lost? #632
Comments
I feel like between this and the other issue you filed, this makes a good case for switching fetches to bidi streams. |
Individual Comment: I was thinking a bit about N control streams. The main argument against it has been ordering would not be guaranteed for important sequences of operations, but I think that could be handled with a control message sequence number. I guess that opens a new flow control problem though. |
I'm not sure what N control streams would do to help the state tracking problem. |
Individual Comment: Sorry, that was a tangent -- FETCH is a control message, so putting it on a bidi stream implicitly makes N control streams. Once we do that, it's tempting to put, say, SUBSCRIBE/SUBSCRIBE_OK/SUBSCRIBE_ERROR/SUBSCRIBE_UPDATE for a given Subscribe ID also on a bidi stream, as well as other message groups (ANNOUNCE*, SUBSCRIBE_ANNOUNCES*). |
That is one possible direction, though I'm not sure that it makes much sense to me. A fetch is well-suited for this since everything can happen on a single stream; subscribes span multiple streams, making the advantage of simplifying the state management much less appealing. |
I agree with Victor that the case for FETCH on a single stream is much cleaner than SUBSCRIBE. |
I have the following test case where the publisher makes an API error and publishes an object on a fetch stream with the wrong length:
C->S: FETCH
S->C: FETCH_OK
S->C: open fetch stream (2),
S->C: write FETCH_HEADER (2, delayed/lost)
S: API error, RESET_STREAM (2)
The client is expecting a fetch stream that will never come.
The client needs to have a timer on the arrival of the FETCH_HEADER in any case, because a malicious or buggy server might never send one on purpose. We should add text to this effect.
We can also short circuit the timer in cases like the above by using RESET_STREAM_AT (always deliver the FETCH_HEADER).
The text was updated successfully, but these errors were encountered: