-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Cluster: Proactively reconfigure when we hit a MOVED response #2286
Conversation
I'm not 100% sure about this because if there is a MOVED happening (e.g. bad proxy somewhere) this would just continually re-run...but only once every 5 seconds. Overall though, we linger in a bad state retrying moves until a discovery happens today and this could be resolved much faster. Meant to help address #1520, #1660, #2074, and #2020.
Making this speedy doesn't help if it's flaky in CI.
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.
I'm fine with the hard-coded 5s as an initial; we can revisit based on feedback and knowledge in the future, if appropriate
….Redis into craver/moved-proactive
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.
FWIW I started reviewing this a couple weeks ago and apparently forgot to submit it - had the same opinion as Marc - this should hopefully be niche enough that it doesn't come up too often, but I think 5 seconds is a perfectly reasonable default setting, and if needed we could always make it configurable in the future.
Meant to help address #1520, #1660, #2074, and #2020.
I'm not 100% sure about this because if there is a MOVED happening (e.g. bad proxy somewhere) this would just continually re-run...but only once every 5 seconds. Overall though, we linger in a bad state retrying moves until a discovery happens today and this could be resolved much faster.