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

Treat ConsumeFailed as a problem #1852

Merged
merged 1 commit into from
Aug 5, 2019
Merged

Conversation

huntc
Copy link
Contributor

@huntc huntc commented Aug 3, 2019

When we get a ConsumeFailed exception in the Consumer then this is a problem that we cannot recover from. To recap, ConsumeFailed is when we've received a Publish from a remote and we need to acknowledge it, but our calling code hasn't yet done so. The problem should cause the client to shutdown in the same way that subscription, or unsubscription does when they fail. Same for the server.

This fix should address a long-standing issue we've had on a super slow machine where ConsumeFailed occurs from time to time, and the stream effectively hangs because the caller wasn't made aware of the problem and couldn't restart the connection. Local testing confirms the fix should work.

The fix also replaces the use of watchWith with just watch as the former provides no means of discerning exceptions (reported akka/akka#26296).

When we get a ConsumeFailed exception in the Consumer, this is a problem. The problem should cause the client to shutdown in the same way that subscription, or unsubscription should fail. Same for the server.

This fix addresses a long-standing issue we've had on a super slow machine where ConsumeFailed occurs from time to time, and the stream effectively hangs because the calling wasn't made aware of the problem and couldn't restart the connection etc.
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ennru ennru merged commit fc4974a into akka:master Aug 5, 2019
@ennru ennru added this to the 1.1.1 milestone Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants