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

Fix redis-trib.rb will fail when migrate a slot with key. #4410

Closed
wants to merge 1 commit into from
Closed

Fix redis-trib.rb will fail when migrate a slot with key. #4410

wants to merge 1 commit into from

Conversation

immortal-boy
Copy link

@immortal-boy immortal-boy commented Nov 1, 2017

The redis-trib.rb call the wrong function which is command client with empty arguments when migrate keys in a slot.

The redis-trib.rb call the wrong function which is command client with empty agruments when migrate keys in a slot.
@kd-devops
Copy link

kd-devops commented Dec 8, 2017

@immortal-boy I am still facing this issue after applying this fix.

I am using redis-4.0.6 with redis-trib-pass.rb and still facing issue while doing redis-trib-pass.rb reshard

[ERR] Calling MIGRATE: ERR Target instance replied with error: NOAUTH Authentication required.

I am passing password using auth flag:

Command Executed :

redis-trib-pass.rb reshard --auth host:ip

This error only comes when cluster nodes has keys ...for empty masters ( or for specifically for empty slots) resharding works fine with auth.

Any suggestion ?

@artix75
Copy link
Contributor

artix75 commented Dec 19, 2017

@immortal-boy Could you provide more details on the issue you're trying to fix with this PR?

@immortal-boy
Copy link
Author

@artix75 I use tcpdump catch network packet when using redis-trib.rb to migrate a slot with keys. And I find redis-trib.rb didn't send command MIGRATE but CLIENT, so I find source.r.client isn't a object to client but just call API function "client".

@immortal-boy
Copy link
Author

@dineshk8666 I don't see the script redis-trib-pass.rb in tag 4.0.6. Where did you get this script?

@artix75
Copy link
Contributor

artix75 commented Jan 4, 2018

@immortal-boy Yes, actually using r.client could be ambiguous. I tried by myself and it worked with me (the r.client method correctly returned the internal ruby object), but i think that it depends on which version of the redis ruby gem is used.
So calling the method ".call" directly on the redis object should remove any ambiguity and the PR is correct.
Which version of the redis ruby gem are you using?
I think that in the future it could be useful to use bundler os something similar in order to always have the same version of the gem.
I'll talk to @antirez about this.

Copy link

@cqyisbug cqyisbug left a comment

Choose a reason for hiding this comment

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

hello,@artix75 ,i cant't figure it out.
i add a sub command in add-node ,url https://github.com/marscqy/redis-in-k8s/blob/master/images/redis-trib.rb
it will do reshard after add a node when the new node becoming a master.
i also modify the redis-trib.rb in my pc, #4771 ,and when i using this sub command to add-node ,i will tell me that the cluster is down .

Copy link
Collaborator

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

we not don't use rb anymore, so i am closing it, if you find other problem in the new code, feel free to open a new one, thanks.

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.

5 participants