diff --git a/README.md b/README.md index 528a0b894c..a29c6ae134 100644 --- a/README.md +++ b/README.md @@ -197,8 +197,8 @@ to maximum value, provided in milliseconds. * `connect_timeout` defaults to `86400000`. Setting `connect_timeout` limits total time for client to reconnect. Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen exactly at the timeout time. That way the default is to try reconnecting until 24h passed. -* `max_attempts` defaults to `null`. By default client will try reconnecting until connected. Setting `max_attempts` -limits total amount of reconnects. Setting this to 0 will prevent any reconnect tries. +* `max_attempts` defaults to `0`. By default client will try reconnecting until connected. Setting `max_attempts` +limits total amount of connection tries. Setting this to 1 will prevent any reconnect tries. * `auth_pass` defaults to `null`. By default client will try connecting without auth. If set, client will run redis auth command on connect. * `family` defaults to `IPv4`. The client connects in IPv4 if not specified or if the DNS resolution returns an IPv4 address. You can force an IPv6 if you set the family to 'IPv6'. See nodejs net or dns modules how to use the family type. diff --git a/index.js b/index.js index e20577a684..25ebb5c015 100644 --- a/index.js +++ b/index.js @@ -52,10 +52,7 @@ function RedisClient(stream, options) { this.should_buffer = false; this.command_queue_high_water = this.options.command_queue_high_water || 1000; this.command_queue_low_water = this.options.command_queue_low_water || 0; - this.max_attempts = null; - if (options.max_attempts || options.max_attempts === 0 || options.max_attempts === '0') { - this.max_attempts = +options.max_attempts; - } + this.max_attempts = +options.max_attempts || 0; this.command_queue = new Queue(); // holds sent commands to de-pipeline them this.offline_queue = new Queue(); // holds commands issued but not able to be sent this.commands_sent = 0; @@ -418,16 +415,6 @@ RedisClient.prototype.connection_gone = function (why) { return; } - if (this.max_attempts !== null && this.attempts > this.max_attempts) { - this.emit('error', new Error("Redis connection in broken state: maximum connection attempts exceeded.")); - return; - } - - if (this.retry_totaltime >= this.connect_timeout) { - this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded.")); - return; - } - debug("Redis connection is gone from " + why + " event."); this.connected = false; this.ready = false; @@ -450,16 +437,23 @@ RedisClient.prototype.connection_gone = function (why) { this.emitted_end = true; } - this.flush_and_error("Redis connection gone from " + why + " event."); - // If this is a requested shutdown, then don't retry if (this.closing) { - this.retry_timer = null; - debug("Connection ended from quit command, not retrying."); + debug("connection ended from quit command, not retrying."); + this.flush_and_error("Redis connection gone from " + why + " event."); + return; + } + + if (this.max_attempts !== 0 && this.attempts >= this.max_attempts || this.retry_totaltime >= this.connect_timeout) { + this.flush_and_error("Redis connection gone from " + why + " event."); + this.end(); + var message = this.retry_totaltime >= this.connect_timeout ? + 'connection timeout exceeded.' : + 'maximum connection attempts exceeded.'; + this.emit('error', new Error("Redis connection in broken state: " + message)); return; } - this.retry_delay = Math.floor(this.retry_delay * this.retry_backoff); if (this.retry_max_delay !== null && this.retry_delay > this.retry_max_delay) { this.retry_delay = this.retry_max_delay; } else if (this.retry_totaltime + this.retry_delay > this.connect_timeout) { @@ -479,6 +473,7 @@ RedisClient.prototype.connection_gone = function (why) { self.retry_totaltime += self.retry_delay; self.attempts += 1; + self.retry_delay = Math.round(self.retry_delay * self.retry_backoff); self.stream = net.createConnection(self.connectionOption); self.install_stream_listeners(); @@ -834,7 +829,7 @@ RedisClient.prototype.end = function () { //clear retry_timer if(this.retry_timer){ clearTimeout(this.retry_timer); - this.retry_timer=null; + this.retry_timer = null; } this.stream.on("error", function(){}); diff --git a/test/connection.spec.js b/test/connection.spec.js index d76a059941..d29214ce1a 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -11,7 +11,7 @@ describe("on lost connection", function () { describe("using " + parser + " and " + ip, function () { it("emit an error after max retry attempts and do not try to reconnect afterwards", function (done) { - var max_attempts = 3; + var max_attempts = 4; var client = redis.createClient({ parser: parser, max_attempts: max_attempts @@ -31,7 +31,7 @@ describe("on lost connection", function () { client.on('error', function(err) { if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) { setTimeout(function () { - assert.strictEqual(calls, max_attempts); + assert.strictEqual(calls, max_attempts - 1); done(); }, 1500); } @@ -40,7 +40,7 @@ describe("on lost connection", function () { it("emit an error after max retry timeout and do not try to reconnect afterwards", function (done) { var connect_timeout = 1000; // in ms - var client = redis.createClient({ + client = redis.createClient({ parser: parser, connect_timeout: connect_timeout }); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 9725cce67b..deb79c3af8 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -688,7 +688,7 @@ describe("The node_redis client", function () { describe('true', function () { it("does not return an error and enqueues operation", function (done) { var client = redis.createClient(9999, null, { - max_attempts: 0, + max_attempts: 1, parser: parser }); @@ -710,6 +710,34 @@ describe("The node_redis client", function () { }, 25); }, 50); }); + + it("enqueues operation and keep the queue while trying to reconnect", function (done) { + var client = redis.createClient(9999, null, { + max_attempts: 4, + parser: parser + }); + var i = 0; + + client.on('error', function(err) { + if (err.message === 'Redis connection in broken state: maximum connection attempts exceeded.') { + assert(i, 3); + assert.strictEqual(client.offline_queue.length, 0); + done(); + } + }); + + client.on('reconnecting', function(params) { + i++; + assert.equal(params.attempt, i); + assert.strictEqual(client.offline_queue.length, 1); + }); + + client.set('foo', 'bar', function(err, result) { + assert(i, 3); + assert('Redis connection gone from error event', err.message); + assert.strictEqual(client.offline_queue.length, 0); + }); + }); }); describe('false', function () {