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(CLUSTER): fix cluster not disconnected when called disconnect method #281

Merged
merged 6 commits into from
Apr 10, 2016

Conversation

luin
Copy link
Collaborator

@luin luin commented Apr 9, 2016

Before this change, when disconnecting from a cluster, the nodes will be disconnected one by one. However, when the subscriber is disconnected, a new subscriber will be connected to in -node event, leading the cluster keeping connected.

Related issue: #277

@@ -57,7 +57,7 @@ function Cluster(startupNodes, options) {

var _this = this;
this.connectionPool.on('-node', function (redis) {
if (_this.subscriber === redis) {
if (_this.status === 'ready' && _this.subscriber === redis) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should export possible statuses to an enum to avoid having typos.
something like:

_this.status = Redis.READY

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Enum may not be able to avoid typos IMO. For example, I may wrongly write Redis.READY as Redis.REEDY, and the latter will be an undefined, no exception would be thrown.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, but your IDE will probably detect it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical though, just saying :)

@luin luin merged commit 91998e3 into master Apr 10, 2016
@luin luin deleted the fix/cluster-cannot-disconnect branch April 10, 2016 15:15
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