From f8ebce9a51a39bfea148b02e3f30465b9303919d Mon Sep 17 00:00:00 2001
From: Andrew Houghton <andrew@electricimp.com>
Date: Thu, 5 Mar 2015 09:05:59 -0500
Subject: [PATCH] We need to be more pro-active with client management, since
 node-redis will never emit an end event when calling client.end() (crazy,
 right?). Also add a concept of unmanaged connections, and test it.

---
 README.md    | 20 ++++++++++++++++++++
 index.js     | 29 +++++++++++++++++++----------
 test/test.js | 27 +++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/README.md b/README.md
index f827ef7..5f2a56a 100644
--- a/README.md
+++ b/README.md
@@ -41,6 +41,26 @@ var sentinelClient = sentinel.createClient(endpoints, {role: 'sentinel'});
 
 Where you should also transparently get a reconnection to a new slave/sentinel if the existing one goes down.
 
+## Connection Management ##
+
+By default node-redis-sentinel will attempt to reconnect any existing redis clients when a failover is
+detected. In some cases you'll want to manage your own connection lifetime and tell node-redis-sentinel
+not to maintain a reference to the redis client -- you can pass explicitly set the 'sentinel_managed'
+option to false when creating a client:
+
+```javascript
+var sentinel = require('redis-sentinel');
+
+var endpoints = [
+    {host: '127.0.0.1', port: 26379},
+    {host: '127.0.0.1', port: 26380}
+];
+var masterName = 'mymaster';
+
+var Sentinel = sentinel.Sentinel(endpoints);
+var masterClient = Sentinel.createClient(masterName, {sentinel_managed: false});
+```
+
 ## TODO ##
 * We could probably be cleverer with reconnects etc. and there may be issues with the error handling
 
diff --git a/index.js b/index.js
index cb719ca..8a578bc 100644
--- a/index.js
+++ b/index.js
@@ -46,7 +46,7 @@ Sentinel.prototype.createClient = function(masterName, opts) {
         self.pubsub.push(pubsubClient);
     }
     return this.createClientInternal(masterName, opts);
-}
+};
 
 Sentinel.prototype.createClientInternal = function(masterName, opts) {
     if (typeof masterName !== 'string') {
@@ -56,25 +56,34 @@ Sentinel.prototype.createClientInternal = function(masterName, opts) {
 
     opts = opts || {};
     var role = opts.role || 'master';
+    var sentinel_managed = opts.sentinel_managed !== false;
+    delete opts['sentinel_managed'];
 
     var endpoints = this.endpoints;
 
 
     var netClient = new net.Socket();
     var client = new redis.RedisClient(netClient, opts);
-    this.clients.push(client);
 
     var self = this;
 
-    client.on('end', function() {
-        // if we're purposefully ending, forget us
-        if (this.closing) {
-            var index = self.clients.indexOf(this);
-            if (index !== -1) {
-                self.clients.splice(index, 1);
+    if (sentinel_managed) {
+        this.clients.push(client);
+        client.on('end', function() {
+            // if we're purposefully ending, forget us
+            if (this.closing) {
+                var index = self.clients.indexOf(this);
+                if (index !== -1) {
+                    self.clients.splice(index, 1);
+                }
             }
-        }
-    });
+        });
+    }
+
+    // clients who call end() won't trigger an end event (node-redis forcibly clears the event
+    // observers from the underlying stream on client.end()); we need to regularly filter for
+    // closing connections, and this is a reasonable place
+    this.clients = this.clients.filter(function(client) { return !client.closing; });
 
     function connectClient(resolver) {
         return function(err, host, port) {
diff --git a/test/test.js b/test/test.js
index 99a8528..38d5f1f 100644
--- a/test/test.js
+++ b/test/test.js
@@ -223,5 +223,32 @@ describe('Redis Sentinel tests', function() {
                 });
             });
         });
+
+        it('should not manage unmanaged clients', function (done) {
+            var endpoints = [{host: '127.0.0.1', port: 26380}];
+            var instance = sentinel.Sentinel(endpoints);
+            var redisClient1 = instance.createClient('mymaster');
+            redisClient1.on('ready', function () {
+                // one pubsub, one actual
+                expect(instance.clients.length).to.equal(2);
+
+                var redisClient2 = instance.createClient('mymaster', {sentinel_managed: false});
+                redisClient2.on('ready', function () {
+                    expect(instance.clients.length).to.equal(2);
+
+                    redisClient1.info(function(err, info) {
+                        expect(err).to.be.null;
+                        expect(info).to.be.ok;
+
+                        redisClient2.info(function(err, info) {
+                            expect(err).to.be.null;
+                            expect(info).to.be.ok;
+
+                            done();
+                        });
+                    });
+                });
+            });
+        });
     });
 });