-
Notifications
You must be signed in to change notification settings - Fork 548
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
Refactor cluster error handling #844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a high-level review, I didn't go into each line.
I think it's a great change, I'll be happy to see it land soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a high-level review, I didn't go into each line.
I think it's a great change, I'll be happy to see it land soon.
ebda2c7
to
7751c0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good. mostly minor comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good. mostly minor comments.
This make some changes to how errors are handled in the redis cluster clients, most notably: * Remove exclusions -- in practice, exclusions create a lot of unnecessary connection rebuilding/slot refreshes when a failed command results in a `MOVED` error. * Attempt to refresh failed connections when IO errors occur instead of adding exclusions and ultimately refreshing slots due to the above-described scenario * Only retry given a subset of error types, otherwise return error immediately.
LGTM |
This make some changes to how errors are handled in the redis cluster clients, most notably:
MOVED
error.