fix: don't add cluster.info to the failover queue before ready

That prevent multiple "ready" event being triggered after the cluster is connected.
redis · Jun 23, 2018 · 491546d · 491546d
1 parent 0d7cc7e
commit 491546d
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 4 deletions.
diff --git a/lib/cluster/connection_pool.js b/lib/cluster/connection_pool.js
@@ -46,6 +46,7 @@ ConnectionPool.prototype.findOrCreate = function (node, readOnly) {
     redis = this.nodes.all[node.key];
     if (redis.options.readOnly !== readOnly) {
       redis.options.readOnly = readOnly;
+      debug('Change role of %s to %s', node.key, readOnly ? 'slave' : 'master')
       redis[readOnly ? 'readonly' : 'readwrite']().catch(_.noop);
       if (readOnly) {
         delete this.nodes.master[node.key];
@@ -56,6 +57,7 @@ ConnectionPool.prototype.findOrCreate = function (node, readOnly) {
       }
     }
   } else {
+    debug('Connecting to %s as %s', node.key, readOnly ? 'slave' : 'master');
     redis = new Redis(_.defaults({
       retryStrategy: null,
       readOnly: readOnly
@@ -122,7 +124,6 @@ ConnectionPool.prototype.reset = function (nodes) {
   });
   Object.keys(newNodes).forEach(function (key) {
     var node = newNodes[key];
-    debug('Connecting to %s as %s', key, node.readOnly ? 'slave' : 'master');
     _this.findOrCreate(node, node.readOnly);
   });
 };

diff --git a/lib/cluster/index.js b/lib/cluster/index.js
@@ -150,7 +150,10 @@ Cluster.prototype.connect = function () {
       if (this.options.enableReadyCheck) {
         this._readyCheck(function (err, fail) {
           if (err || fail) {
-            this.disconnect(true);
+            debug('Ready check failed (%s). Reconnecting...', err || fail)
+            if (this.status === 'connect') {
+              this.disconnect(true);
+            }
           } else {
             readyHandler.call(this);
           }
@@ -195,7 +198,9 @@ Cluster.prototype._handleCloseEvent = function () {
     this.reconnectTimeout = setTimeout(function () {
       this.reconnectTimeout = null;
       debug('Cluster is disconnected. Retrying after %dms', retryDelay);
-      this.connect().catch(_.noop);
+      this.connect().catch(function (err) {
+        debug('Got error %s when reconnecting. Ignoring...', err);
+      });
     }.bind(this), retryDelay);
   } else {
     this.setStatus('end');
@@ -218,6 +223,7 @@ Cluster.prototype.disconnect = function (reconnect) {
   if (this.reconnectTimeout) {
     clearTimeout(this.reconnectTimeout);
     this.reconnectTimeout = null;
+    debug('Canceled reconnecting attempts');
   }
 
   if (status === 'wait') {
@@ -570,7 +576,7 @@ Cluster.prototype.handleError = function (error, ttl, handlers) {
       timeout: this.options.retryDelayOnClusterDown,
       callback: this.refreshSlotsCache.bind(this)
     });
-  } else if (error.message === utils.CONNECTION_CLOSED_ERROR_MSG && this.options.retryDelayOnFailover > 0) {
+  } else if (error.message === utils.CONNECTION_CLOSED_ERROR_MSG && this.options.retryDelayOnFailover > 0 && this.status === 'ready') {
     this.delayQueue.push('failover', handlers.connectionClosed, {
       timeout: this.options.retryDelayOnFailover,
       callback: this.refreshSlotsCache.bind(this)