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

Enable refresh slots after reconnecting to initial nodes. #2921

Merged

Conversation

GilboaAWS
Copy link
Collaborator

@GilboaAWS GilboaAWS commented Jan 7, 2025

Moving to not throttle refresh slots after reconnecting to initial nodes, as this operation is kind of 'resetting' the client, so it should let it discover the whole topology before moving on to handle requests.

…as this operation is kind of resetting the client, so it should let it discover the whole topology before moving on the handle requests

Signed-off-by: GilboaAWS <[email protected]>
@GilboaAWS GilboaAWS requested a review from a team as a code owner January 7, 2025 10:23
Copy link
Collaborator

@barshaul barshaul left a comment

Choose a reason for hiding this comment

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

LGTM: The reason it was passed as ‘throttable’ was that we encountered this error not only when all connections were unavailable. We needed safety measures to ensure we didn’t overload the server. Since this has been fixed, and AllConnectionsUnavailable now serves only to 'reset' the client, this modification is appropriate.

@GilboaAWS GilboaAWS merged commit b6565eb into valkey-io:main Jan 7, 2025
58 checks passed
@GilboaAWS GilboaAWS deleted the refresh_slot_after_reconnect_initials branch January 7, 2025 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants