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.

Sign up for GitHub

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

Jump to bottom

Fix issues with slot parsing in cluster connections. #97

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Fix issues with slot parsing in cluster connections. #97

merged 4 commits into from
Jan 9, 2024

Conversation

nihohit
Copy link

@nihohit nihohit commented Jan 4, 2024 •
edited by shachlanAmazon
Loading

Issue #, if available:
valkey-io/valkey-glide#659
valkey-io/valkey-glide#532
valkey-io/valkey-glide#430

Description of changes:
based over #95

  1. When parsing slots, now the slots will be sorted, to ensure that they're ordered the same way regardless of the order returned from the server.
  2. When parsing slots, if hostname is missing, fill it in with the queried node's hostname.
  3. Hash the topology view according to the parsed slots instead of the returned values, in order to account for the 2 fixes in the hash.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

nihohit added 3 commits January 4, 2024 17:46
@nihohit
Sort slots after parsing them.
8acb1c1
@nihohit
Use helper function to clean up tests.
c80e5d2
@nihohit
Hash topology views according to parsed slots.
d317ab7 
This means that the hash will take into account our fixes to the parsed
slots, and so will be consistent even if different nodes return the
same view differently.
ikolomi
ikolomi approved these changes Jan 9, 2024
View reviewed changes
Copy link

@ikolomi ikolomi left a comment

Choose a reason for hiding this comment

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

Please make sure this solves the panic of valkey-io/valkey-glide#659

@shachlanAmazon shachlanAmazon merged commit abdce8c into amazon-contributing:main Jan 9, 2024
10 checks passed
@nihohit nihohit deleted the slots-view branch January 9, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikolomi ikolomi ikolomi approved these changes

@barshaul barshaul Awaiting requested review from barshaul

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nihohit @ikolomi @shachlanAmazon