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

Replace CLUSTER SLOTS with CLUSTER NODES to get slot map #64

Merged
merged 4 commits into from
Jun 2, 2016

Conversation

maralla
Copy link
Contributor

@maralla maralla commented May 26, 2016

No description provided.

@jasonjoo2010
Copy link
Contributor

hi, maralla

why we considered to replace it?

the output of slots command is structured while nodes' is unsure and is mainly for reading i think.
if the slots is fully distributed it is not sure whether nodes command is reliable.

eg.:
master - 0 1464406084408 14 connected 1,3,4,5,6,8192-12287,13223

is it for network transfer reasons?

@maralla
Copy link
Contributor Author

maralla commented May 30, 2016

CLUSTER SLOTS is much slower than CLUSTER NODES in large clusters with many discrete slots.

@jasonjoo2010
Copy link
Contributor

yeah got it
it is suggested add fully test case which has many seperated slots associated.

@maralla maralla force-pushed the cluster-nodes branch 8 times, most recently from 1de04fd to af4990a Compare June 1, 2016 07:32
@maralla maralla force-pushed the cluster-nodes branch 4 times, most recently from e7e9f03 to d6fb443 Compare June 2, 2016 08:00
@maralla maralla merged commit 3151c5f into eleme:master Jun 2, 2016
@maralla maralla deleted the cluster-nodes branch June 2, 2016 08:58
@wooparadog
Copy link
Contributor

@jasonjoo2010 Another user has reported that because redis 3.2.0 changed the return value of cluster slots, corvus@0.2.3 is not able to update its slot information. So using cluster nodes fixed that as a bonus...

wooparadog referenced this pull request Jun 7, 2016
Corvus==0.2.3 doesn't support redis 3.2.0 yet
@doyoubi doyoubi mentioned this pull request Apr 3, 2017
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.

None yet

3 participants