fetch()/next() rely on missed heartbeats to detect abnormally ending next requests #150
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
…to fail a requests that were not properly "finished" by the server. This has the benefit that messages arriving "late" are now likely to complete successfully since the client won't be proactively ending the request. change(jetstream) non-ordered fetch/next will now throw/reject if heartbeats are missed - previously you could find out if you had statuses enabled on the fetch. On next() it was silently ignored and the client received a null message. tests(jetstream): removed flapper tests that attempted yield ConsumerNotFound/StreamNotFound when a resource as removed prior to a consume/fetch/next. tests(jestream): fixed cross-account configurations to have a response_type of "stream" There's also a small implication for the edge case where a client is partitioned from jetstream towards the end of the fetch/next - in those cases is possible for the request to extend for up-to a minute (max heartbeat interval is 30s, and two misses are required). This is an edge condition when compared to the normal fetch/next that gets a new message towards the end of the expires. In this more common case, there won't be a redelivery since the message won't be dropped because of an overly eager timeout. Fix #149 Signed-off-by: Alberto Ricart <[email protected]>
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.
fix(jetstream) fetch/next() now rely on missing heartbeat detection to fail requests that were not properly "finished" by the server. This has the benefit that messages arriving "late" are now likely to complete successfully since the client won't be proactively ending the request.
change(jetstream) non-ordered fetch/next will now throw/reject if heartbeats are missed - previously you could find out if you had statuses enabled on the fetch. On next() it was silently ignored and the client received a null message.
tests(jetstream): removed flapper tests that attempted yield ConsumerNotFound/StreamNotFound when a resource as removed prior to a consume/fetch/next.
tests(jestream): fixed cross-account configurations to have a response_type of "stream"
There's also a small implication for the edge case where a client is partitioned from jetstream towards the end of the fetch/next - in those cases is possible for the request to extend for up-to a minute (max heartbeat interval is 30s, and two misses are required). This is an edge condition when compared to the normal fetch/next that gets a new message towards the end of the expires. In this more common case, there won't be a redelivery since the message won't be dropped because of an overly eager timeout.
Fix #149