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

Resolves a bug with cluster where a subscribe is sent to a disconnected node #63

Merged
merged 1 commit into from
Jun 5, 2015

Conversation

devaos
Copy link
Contributor

@devaos devaos commented Jun 4, 2015

The issue came up when connecting to a load balancer, sitting in front of the redis cluster nodes, and attempting to subscribe as soon as the 'ready' event was fired. The load balancer's IP:port is not in redis cluster's list of nodes so the connection was disconnected and removed, causing the subscribe to be sent to the disconnected node.

@luin
Copy link
Collaborator

luin commented Jun 4, 2015

When a node is disconnected(https://github.com/luin/ioredis/blob/master/lib/cluster.js#L407), ioredis will re-select a new subscriber automatically(https://github.com/luin/ioredis/blob/master/lib/cluster.js#L164). So you don't need to call selectSubscriber manually.

@devaos
Copy link
Contributor Author

devaos commented Jun 4, 2015

Oh, I see what you mean. However this commit did resolve the issue we were having. I'll comment shortly with the debug logs so you can see what was happening.

@devaos devaos force-pushed the master branch 3 times, most recently from 2b5cccc to 1cb1575 Compare June 4, 2015 19:42
@devaos
Copy link
Contributor Author

devaos commented Jun 4, 2015

Here is a snippet of the debug output before the patch. I added a couple console lines to clarify which command was being sent and whether the stream was writable.

  ioredis:redis status[127.0.0.1:6381]: close -> end +1ms
  ioredis:redis status[10.240.14.47:6375]: connect -> ready +0ms
  ioredis:redis status[10.240.46.240:6375]: connect -> ready +0ms
  ioredis:redis status[10.240.75.76:6375]: connect -> ready +1ms
  ioredis:redis status[127.0.0.1:6381]: ready -> close +0ms
  ioredis:connection skip reconnecting since the connection is manually closed. +0ms
  ioredis:redis status[127.0.0.1:6381]: close -> end +0ms
  ioredis:cluster getting slot cache from 10.240.46.240:6375 +2s
sendCommand: command.name=cluster
sendCommand: stream.writable=undefined
sendCommand: this.stream.writable=true
  ioredis:redis write command[0] -> cluster(slots) +1ms
sendCommand: command.name=subscribe
sendCommand: stream.writable=undefined
sendCommand: this.stream.writable=false
  ioredis:cluster getting slot cache from 10.240.46.240:6375 +2s
sendCommand: command.name=cluster
sendCommand: stream.writable=undefined
sendCommand: this.stream.writable=true
  ioredis:redis write command[0] -> cluster(slots) +0ms
sendCommand: command.name=subscribe
sendCommand: stream.writable=undefined
sendCommand: this.stream.writable=false

@devaos
Copy link
Contributor Author

devaos commented Jun 4, 2015

I updated the commit with what I believe is a more appropriate fix based on your feedback. Thanks

luin added a commit that referenced this pull request Jun 5, 2015
Resolves a bug with cluster where a subscribe is sent to a disconnected node
@luin luin merged commit 083c7c2 into redis:master Jun 5, 2015
@luin
Copy link
Collaborator

luin commented Jun 5, 2015

Awesome! Thank you for the patch!

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.

2 participants