chore: strange case not closing stream #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
menduz
commented
Jun 3, 2022
Comment on lines
+96
to
+114
| /** | ||
| * If a StreamMessage is received, then it means we have the POSSIBILITY to | ||
| * consume a remote generator. The client must answer every ACK with the next | ||
| * inteded action, could be: next(), close(). Both actions are serialized in the | ||
| * StreamMessage. The server MUST NOT generate any new element of the generator | ||
| * if the client doesn't ask for it. | ||
| * | ||
| * The whole protocol is designed to be SLOW AND SECURE, that means, ACKs (slow) | ||
| * will block the generation and consumption of iterators (secure). | ||
| * | ||
| * That exist to save the memory of the servers and to generate the much needed | ||
| * backpressure. | ||
| * | ||
| * If throughput is what you are looking for, you may better use bigger messages | ||
| * containing serialized lists. Effectively reducing the number of messages | ||
| * and increasing their size. | ||
| * | ||
| * @internal | ||
| */ |
leanmendoza
reviewed
Jun 3, 2022
| S-->C: StreamMessage {message_id,payload,seqId=2} | ||
| C->S: StreamMessage {ack=true,message_id,seqId=2,closed=true} | ||
| S->S: Close async Generator | ||
| C->C: Close async Iterator |
There was a problem hiding this comment.
should this close async iterator be here?
There was a problem hiding this comment.
I mean the C->C: Close async Iterator is duplicated and I think the last one is the right.
There was a problem hiding this comment.
the server has a generator, the client has an iterator. I think it is sound
leanmendoza
approved these changes
Jun 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.