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

Extend SlotLoader to work with multiple slot ranges #894

Merged
merged 1 commit into from Mar 23, 2020
Merged

Conversation

@rahul342
Copy link
Contributor

rahul342 commented Mar 22, 2020

With multiple resharding, different slot ranges can be assigned to a
node.
e.g.
Screenshot 2020-03-22 02 23 01

Current logic only preassigns the last slot range, causing errors & retries for unassigned slots.
This commit makes it work with multiple slot ranges and assigning all slots.

I tested this locally, but couldn't figure how to add tests for this scenario.
edit: Added partial tests.

@byroot
Copy link
Member

byroot commented Mar 22, 2020

Code looks sane to me. I'll merge once you add the test @supercaracal required.

With multiple resharding, different slot ranges can be assigned to a
node. Current logic only pre assigns the last slot range, causing errors
& retries for unassigned slots.
This commit makes it work with multiple slot ranges and assigning all
slots.
@rahul342 rahul342 force-pushed the rahul342:master branch from af6f8bb to 5849cb2 Mar 23, 2020
@rahul342
Copy link
Contributor Author

rahul342 commented Mar 23, 2020

@byroot done

@rahul342
Copy link
Contributor Author

rahul342 commented Mar 23, 2020

cc @cliu55

@byroot byroot merged commit 7127f3b into redis:master Mar 23, 2020
17 of 21 checks passed
17 of 21 checks passed
Main (ubuntu-latest, 5.0, 2.7, ruby)
Details
Main (ubuntu-latest, 5.0, 2.7, hiredis)
Details
Main (ubuntu-latest, 5.0, 2.7, synchrony)
Details
Main (ubuntu-latest, 5.0, 2.6, ruby)
Details
Main (ubuntu-latest, 5.0, 2.6, hiredis)
Details
Main (ubuntu-latest, 5.0, 2.6, synchrony)
Details
Main (ubuntu-latest, 5.0, 2.5, ruby)
Details
Main (ubuntu-latest, 5.0, 2.5, hiredis)
Details
Main (ubuntu-latest, 5.0, 2.5, synchrony)
Details
Main (ubuntu-latest, 5.0, 2.4, ruby)
Details
Main (ubuntu-latest, 5.0, 2.4, hiredis)
Details
Main (ubuntu-latest, 5.0, 2.4, synchrony)
Details
Sub (ubuntu-latest, 5.0, jruby-9.2.9.0, ruby) Sub (ubuntu-latest, 5.0, jruby-9.2.9.0, ruby)
Details
Sub (ubuntu-latest, 5.0, 2.3, ruby)
Details
Sub (ubuntu-latest, 4.0, jruby-9.2.9.0, ruby) Sub (ubuntu-latest, 4.0, jruby-9.2.9.0, ruby)
Details
Sub (ubuntu-latest, 4.0, 2.3, ruby)
Details
Sub (ubuntu-latest, 3.2, jruby-9.2.9.0, ruby) Sub (ubuntu-latest, 3.2, jruby-9.2.9.0, ruby)
Details
Sub (ubuntu-latest, 3.2, 2.3, ruby)
Details
Sub (ubuntu-latest, 3.0, jruby-9.2.9.0, ruby) Sub (ubuntu-latest, 3.0, jruby-9.2.9.0, ruby)
Details
Sub (ubuntu-latest, 3.0, 2.3, ruby)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@byroot
Copy link
Member

byroot commented Mar 23, 2020

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.