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

Standalone Client: Raise an error if more than one primary node is found #1487

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

barshaul
Copy link
Collaborator

The standalone client receives a list of node addresses and tries to find the primary by getting the node's role from 'info replication'. In some cluster issues there might be more than a single primary so the client won't be able to identify which one is the legit primary, and therefore we should fail the client creation.

@barshaul barshaul requested a review from a team as a code owner May 29, 2024 12:12
@barshaul barshaul marked this pull request as draft May 29, 2024 12:14
@barshaul barshaul marked this pull request as ready for review May 29, 2024 13:14
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

How to reproduce and test it manually?
Won't GLIDE be confused if a cluster lost a replica or few nodes? It may come into the same situation: 2 primary + 1 replica.

glide-core/src/client/standalone_client.rs Outdated Show resolved Hide resolved
glide-core/src/client/standalone_client.rs Show resolved Hide resolved
@Yury-Fridlyand Yury-Fridlyand added the Rust core redis-rs/glide-core matter label May 29, 2024
glide-core/src/client/reconnecting_connection.rs Outdated Show resolved Hide resolved
glide-core/src/client/standalone_client.rs Outdated Show resolved Hide resolved
glide-core/src/client/standalone_client.rs Outdated Show resolved Hide resolved
glide-core/src/client/standalone_client.rs Outdated Show resolved Hide resolved
@barshaul
Copy link
Collaborator Author

@eifrah-aws round

@barshaul
Copy link
Collaborator Author

How to reproduce and test it manually? Won't GLIDE be confused if a cluster lost a replica or few nodes? It may come into the same situation: 2 primary + 1 replica.

  1. You can create two primaries and create with their addresses a standalone client
  2. I don't understand the scenario..? how loosing replicas will end up with multiple primaries?

@barshaul barshaul merged commit 1403456 into valkey-io:main Jun 3, 2024
46 checks passed
@barshaul barshaul deleted the primary_conflict branch June 3, 2024 15:09
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust core redis-rs/glide-core matter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants