JedisSentinelPool now takes care of multi failover

* Now Pool.initPool() call closeInternalPool(), instead of destroy()
** calling destroy() in Pool.initPool() may have side effect, and JedisSentinelPool did
* modify unit test to test failover twice (needs +1 slave)
** modify configurations for additional slave

Makefile

@@ -39,6 +39,17 @@ save ""
 appendonly no
 endef
 
+define REDIS5_CONF
+daemonize yes
+port 6383
+requirepass foobared
+masterauth foobared
+pidfile /tmp/redis5.pid
+logfile /tmp/redis5.log
+save ""
+appendonly no
+endef
+
 define REDIS_SENTINEL1
 port 26379
 daemonize yes
@@ -82,6 +93,7 @@ export REDIS1_CONF
 export REDIS2_CONF
 export REDIS3_CONF
 export REDIS4_CONF
+export REDIS5_CONF
 export REDIS_SENTINEL1
 export REDIS_SENTINEL2
 export REDIS_SENTINEL3
@@ -91,16 +103,18 @@ start:
 	echo "$$REDIS2_CONF" | redis-server -
 	echo "$$REDIS3_CONF" | redis-server -
 	echo "$$REDIS4_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL1" | redis-sentinel -
+	echo "$$REDIS5_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL2" | redis-sentinel -
+	echo "$$REDIS_SENTINEL1" | redis-server - --sentinel
-	echo "$$REDIS_SENTINEL3" | redis-sentinel -
+	echo "$$REDIS_SENTINEL2" | redis-server - --sentinel
+	echo "$$REDIS_SENTINEL3" | redis-server - --sentinel
 
 stop:
 	kill `cat /tmp/redis1.pid`
 	kill `cat /tmp/redis2.pid`
 	# this get's segfaulted by the tests
 	kill `cat /tmp/redis3.pid` || true
-	kill `cat /tmp/redis4.pid`
+	kill `cat /tmp/redis4.pid` || true
+	kill `cat /tmp/redis5.pid` || true
 	kill `cat /tmp/sentinel1.pid`
 	kill `cat /tmp/sentinel2.pid`
 	kill `cat /tmp/sentinel3.pid`
@@ -110,9 +124,10 @@ test:
 	echo "$$REDIS2_CONF" | redis-server -
 	echo "$$REDIS3_CONF" | redis-server -
 	echo "$$REDIS4_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL1" | redis-sentinel -
+	echo "$$REDIS5_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL2" | redis-sentinel -
+	echo "$$REDIS_SENTINEL1" | redis-server - --sentinel
-	echo "$$REDIS_SENTINEL3" | redis-sentinel -
+	echo "$$REDIS_SENTINEL2" | redis-server - --sentinel
+	echo "$$REDIS_SENTINEL3" | redis-server - --sentinel
 
 	mvn clean compile test
 
@@ -120,7 +135,8 @@ test:
 	kill `cat /tmp/redis2.pid`
 	# this get's segfaulted by the tests
 	kill `cat /tmp/redis3.pid` || true
-	kill `cat /tmp/redis4.pid`
+	kill `cat /tmp/redis4.pid` || true
+	kill `cat /tmp/redis5.pid` || true
 	kill `cat /tmp/sentinel1.pid`
 	kill `cat /tmp/sentinel2.pid`
 	kill `cat /tmp/sentinel3.pid`
@@ -130,9 +146,10 @@ deploy:
 	echo "$$REDIS2_CONF" | redis-server -
 	echo "$$REDIS3_CONF" | redis-server -
 	echo "$$REDIS4_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL1" | redis-sentinel -
+	echo "$$REDIS5_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL2" | redis-sentinel -
+	echo "$$REDIS_SENTINEL1" | redis-server - --sentinel
-	echo "$$REDIS_SENTINEL3" | redis-sentinel -
+	echo "$$REDIS_SENTINEL2" | redis-server - --sentinel
+	echo "$$REDIS_SENTINEL3" | redis-server - --sentinel
 
 	mvn clean deploy
 
@@ -140,7 +157,8 @@ deploy:
 	kill `cat /tmp/redis2.pid`
 	# this get's segfaulted by the tests
 	kill `cat /tmp/redis3.pid` || true
-	kill `cat /tmp/redis4.pid`
+	kill `cat /tmp/redis4.pid` || true
+	kill `cat /tmp/redis5.pid` || true
 	kill `cat /tmp/sentinel1.pid`
 	kill `cat /tmp/sentinel2.pid`
 	kill `cat /tmp/sentinel3.pid`
@@ -150,9 +168,10 @@ release:
 	echo "$$REDIS2_CONF" | redis-server -
 	echo "$$REDIS3_CONF" | redis-server -
 	echo "$$REDIS4_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL1" | redis-sentinel -
+	echo "$$REDIS5_CONF" | redis-server -
-	echo "$$REDIS_SENTINEL2" | redis-sentinel -
+	echo "$$REDIS_SENTINEL1" | redis-server - --sentinel
-	echo "$$REDIS_SENTINEL3" | redis-sentinel -
+	echo "$$REDIS_SENTINEL2" | redis-server - --sentinel
+	echo "$$REDIS_SENTINEL3" | redis-server - --sentinel
 
 	mvn release:clean
 	mvn release:prepare
@@ -162,7 +181,8 @@ release:
 	kill `cat /tmp/redis2.pid`
 	# this get's segfaulted by the tests
 	kill `cat /tmp/redis3.pid` || true
-	kill `cat /tmp/redis4.pid`
+	kill `cat /tmp/redis4.pid` || true
+	kill `cat /tmp/redis5.pid` || true 
 	kill `cat /tmp/sentinel1.pid`
 	kill `cat /tmp/sentinel2.pid`
 	kill `cat /tmp/sentinel3.pid`

pom.xml

@@ -45,7 +45,7 @@
   </scm>
 
 	<properties>
-		<redis-hosts>localhost:6379,localhost:6380,localhost:6381,localhost:6382</redis-hosts>
+		<redis-hosts>localhost:6379,localhost:6380,localhost:6381,localhost:6382,localhost:6383</redis-hosts>
 		<sentinel-hosts>localhost:26379,localhost:26380</sentinel-hosts>
     <github.global.server>github</github.global.server>
 	</properties>

src/main/java/redis/clients/util/Pool.java

@@ -24,7 +24,7 @@ public abstract class Pool<T> {
     	
     	if (this.internalPool != null) {
     		try {
-    			destroy();
+    			closeInternalPool();
     		} catch (Exception e) {    			
     		}
     	}
@@ -58,7 +58,11 @@ public abstract class Pool<T> {
     public void returnResource(final T resource) {
     	returnResourceObject(resource);
     }
-
+    
+    public void destroy() {
+    	closeInternalPool();
+    }
+    
     protected void returnBrokenResourceObject(final Object resource) {
         try {
             internalPool.invalidateObject(resource);
@@ -68,8 +72,8 @@ public abstract class Pool<T> {
         }
     }
 
-    public void destroy() {
+    protected void closeInternalPool() {
-        try {
+    	try {
             internalPool.close();
         } catch (Exception e) {
             throw new JedisException("Could not destroy the pool", e);

src/test/java/redis/clients/jedis/tests/HostAndPortUtil.java

@@ -33,18 +33,23 @@ public class HostAndPortUtil {
         
         HostAndPort defaulthnp5 = new HostAndPort();
         defaulthnp5.host = "localhost";
-        defaulthnp5.port = Protocol.DEFAULT_SENTINEL_PORT;
+        defaulthnp5.port = Protocol.DEFAULT_PORT + 4;
-        sentinelHostAndPortList.add(defaulthnp5);
+        redisHostAndPortList.add(defaulthnp5);
         
         HostAndPort defaulthnp6 = new HostAndPort();
         defaulthnp6.host = "localhost";
-        defaulthnp6.port = Protocol.DEFAULT_SENTINEL_PORT + 1;
+        defaulthnp6.port = Protocol.DEFAULT_SENTINEL_PORT;
         sentinelHostAndPortList.add(defaulthnp6);
         
         HostAndPort defaulthnp7 = new HostAndPort();
         defaulthnp7.host = "localhost";
-        defaulthnp7.port = Protocol.DEFAULT_SENTINEL_PORT + 2;
+        defaulthnp7.port = Protocol.DEFAULT_SENTINEL_PORT + 1;
         sentinelHostAndPortList.add(defaulthnp7);
+        
+        HostAndPort defaulthnp8 = new HostAndPort();
+        defaulthnp8.host = "localhost";
+        defaulthnp8.port = Protocol.DEFAULT_SENTINEL_PORT + 2;
+        sentinelHostAndPortList.add(defaulthnp8);
 
         String envRedisHosts = System.getProperty("redis-hosts");
         String envSentinelHosts = System.getProperty("sentinel-hosts");

src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java

@@ -18,6 +18,8 @@ public class JedisSentinelPoolTest extends JedisTestBase {
 	    .get(2);
     protected static HostAndPort slave1 = HostAndPortUtil.getRedisServers()
 	    .get(3);
+    protected static HostAndPort slave2 = HostAndPortUtil.getRedisServers()
+    	.get(4);
     protected static HostAndPort sentinel1 = HostAndPortUtil
 	    .getSentinelServers().get(1);
     protected static HostAndPort sentinel2 = HostAndPortUtil
@@ -25,6 +27,9 @@ public class JedisSentinelPoolTest extends JedisTestBase {
 
     protected static Jedis masterJedis;
     protected static Jedis slaveJedis1;
+    protected static Jedis slaveJedis2;
+    
+    protected static int slaveCount = 0;
 
     protected Set<String> sentinels = new HashSet<String>();
 
@@ -39,37 +44,55 @@ public class JedisSentinelPoolTest extends JedisTestBase {
 	slaveJedis1 = new Jedis(slave1.host, slave1.port);
 	slaveJedis1.auth("foobared");
 	slaveJedis1.slaveof(master.host, master.port);
+	slaveCount++;
+	
+	slaveJedis2 = new Jedis(slave2.host, slave2.port);
+	slaveJedis2.auth("foobared");
+	slaveJedis2.slaveof(master.host, master.port);
+	slaveCount++;
 
 	sentinels.add(sentinel1.toString());
 	sentinels.add(sentinel2.toString());
 
 	// FIXME: The following allows the master/slave relationship to
-	// be established. We can do this more elegantly.
+	// be established, and let sentinels know about this relationship. 
+	// We can do this more elegantly.
 	Thread.sleep(10000);
     }
 
     @Test
-    public void segfaultMaster() throws InterruptedException {
+    public void ensureSafeTwiceFailover() throws InterruptedException {
+    	JedisSentinelPool pool = new JedisSentinelPool("mymaster", sentinels,
+    			new Config(), 1000, "foobared", 2);
+    		
+    	// perform failover
+    	doSegFaultMaster(pool);
+    	
+    	// perform failover once again
+    	doSegFaultMaster(pool);
+    	
+    	// you can test failover as much as possible
+    	// but you need to prepare additional slave per failover
+    }
+    
+    private void doSegFaultMaster(JedisSentinelPool pool) throws InterruptedException {
+    	// jedis connection should be master
+    	Jedis jedis = pool.getResource();
+    	assertEquals("PONG", jedis.ping());
 
-	JedisSentinelPool pool = new JedisSentinelPool("mymaster", sentinels,
+    	try {
-		new Config(), 1000, "foobared", 2);
+    		jedis.debug(DebugParams.SEGFAULT());
+    	} catch (Exception e) {
+    	}
 
-	Jedis jedis = pool.getResource();
+    	// wait for the sentinel to promote a master
-	assertEquals("PONG", jedis.ping());
+    	// FIXME: we can query the sentinel and sleep
+    	// right until the master is promoted
+    	Thread.sleep(35000);
 
-	try {
+    	jedis = pool.getResource();
-	    masterJedis.debug(DebugParams.SEGFAULT());
+    	assertEquals("PONG", jedis.ping());
-	} catch (Exception e) {
+    	assertEquals("foobared", jedis.configGet("requirepass").get(1));
-	}
+    	assertEquals(2, jedis.getDB().intValue());
-
-	// wait for the sentinel to promote a master
-	// FIXME: we can query the sentinel and sleep
-	// right until the master is promoted
-	Thread.sleep(35000);
-
-	jedis = pool.getResource();
-	assertEquals("PONG", jedis.ping());
-	assertEquals("foobared", jedis.configGet("requirepass").get(1));
-	assertEquals(2, jedis.getDB().intValue());
     }
 }