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

Improve redis-trib replica assignment #2227

Merged
merged 1 commit into from
Dec 23, 2014

Conversation

mattsta
Copy link
Contributor

@mattsta mattsta commented Dec 20, 2014

This tiny bit of code has gone through so many revisions. Hopefully
it's more correct now.

Fixes #2204

This tiny bit of code has gone through so many revisions.  Hopefully
it's more correct now.

Fixes redis#2204
@badboy
Copy link
Contributor

badboy commented Dec 20, 2014

It is more correct now and catches a few more edge cases. I think we should go for it.
My hope is that in the long-run we can improve the tools even more (when the first people actually deployed this).
For the kind of perfect distribution of master/slaves we are better off relying on a human that pre-calculates everything instead of trying to support each and every edge case with our code.

antirez added a commit that referenced this pull request Dec 23, 2014
@antirez antirez merged commit f7bc1fc into redis:unstable Dec 23, 2014
@antirez
Copy link
Contributor

antirez commented Dec 23, 2014

Thanks! Merged. I believe for redis-trib wi'll soon start to see good external contributors once a reasonable amount of people deploy Redis Cluster.

@mkleins
Copy link

mkleins commented May 15, 2015

I have always the problem for the replica assignement with 6 instances (3 masters/3 slaves).

Adding replica 10.0.2.42:7001 to 10.0.2.41:7000
Adding replica 10.0.2.41:7001 to 10.0.2.42:7000
Adding replica 10.0.2.43:7001 to 10.0.2.43:7000

The master and the slave of the same node are hosted on the same host.

@sidd081-zz
Copy link

@mkleins
I made a small change in redis-trib.rb and it worked fine for me in most of the cases. In def alloc_slots
in script, after masters.each{|m| puts m} add one more new line interleaved.push interleaved.shift

....
579 masters = interleaved.slice!(0, masters_count)
580 nodes_count -= masters.length
581
582 masters.each{|m| puts m}
583 interleaved.push interleaved.shift
....

@alighanem
Copy link

alighanem commented Sep 18, 2017

@sidd081 It worked for me with this correction. It is necessary to integrate it with the master. thks!!

@sidd081-zz
Copy link

thanks @alighanem for trying it out :)

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.

redis-trib.rb may allocate the slave and master in the same host
6 participants