Merge branch 'fast-fail-while-initializing-jedis-sentinel-pool'

src/main/java/redis/clients/jedis/JedisSentinelPool.java

@@ -10,6 +10,7 @@ import java.util.logging.Logger;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
 
 import redis.clients.jedis.exceptions.JedisConnectionException;
+import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.util.Pool;
 
 public class JedisSentinelPool extends Pool<Jedis> {
@@ -112,52 +113,53 @@ public class JedisSentinelPool extends Pool<Jedis> {
 	    final String masterName) {
 
 	HostAndPort master = null;
-	boolean running = true;
+	boolean sentinelAvailable = false;
 
-	outer: while (running) {
+	log.info("Trying to find master from available Sentinels...");
 
-	    log.info("Trying to find master from available Sentinels...");
+	for (String sentinel : sentinels) {
+	    final HostAndPort hap = toHostAndPort(Arrays.asList(sentinel
+		    .split(":")));
 
-	    for (String sentinel : sentinels) {
+	    log.fine("Connecting to Sentinel " + hap);
 
-		final HostAndPort hap = toHostAndPort(Arrays.asList(sentinel
+	    Jedis jedis = null;
-			.split(":")));
+	    try {
+		jedis = new Jedis(hap.getHost(), hap.getPort());
 
-		log.fine("Connecting to Sentinel " + hap);
+		List<String> masterAddr = jedis
+			.sentinelGetMasterAddrByName(masterName);
 
-		Jedis jedis = null;
+		// connected to sentinel...
-		try {
+		sentinelAvailable = true;
-		    jedis = new Jedis(hap.getHost(), hap.getPort());
 
-		    if (master == null) {
+		if (masterAddr == null || masterAddr.size() != 2) {
-			List<String> masterAddr = jedis
+		    log.warning("Can not get master addr, master name: "
-				.sentinelGetMasterAddrByName(masterName);
+			    + masterName + ". Sentinel: " + hap + ".");
-			if (masterAddr == null || masterAddr.size() != 2) {
+		    continue;
-			    log.warning("Can not get master addr, master name: "
+		}
-				    + masterName + ". Sentinel: " + hap + ".");
-			    continue;
-			}
 
-			master = toHostAndPort(masterAddr);
+		master = toHostAndPort(masterAddr);
-			log.fine("Found Redis master at " + master);
+		log.fine("Found Redis master at " + master);
-			break outer;
+		break;
-		    }
+	    } catch (JedisConnectionException e) {
-		} catch (JedisConnectionException e) {
+		log.warning("Cannot connect to sentinel running @ " + hap
-		    log.warning("Cannot connect to sentinel running @ " + hap
+			+ ". Trying next one.");
-			    + ". Trying next one.");
+	    } finally {
-		} finally {
+		if (jedis != null) {
-		    if (jedis != null) {
+		    jedis.close();
-			jedis.close();
-		    }
 		}
 	    }
+	}
 
-	    try {
+	if (master == null) {
-		log.severe("All sentinels down, cannot determine where is "
+	    if (sentinelAvailable) {
-			+ masterName + " master is running... sleeping 1000ms.");
+		// can connect to sentinel, but master name seems to not monitored
-		Thread.sleep(1000);
+		throw new JedisException("Can connect to sentinel, but " + masterName
-	    } catch (InterruptedException e) {
+			+ " seems to be not monitored...");
-		e.printStackTrace();
+	    } else {
+		throw new JedisConnectionException("All sentinels down, cannot determine where is "
+			+ masterName + " master is running...");
 	    }
 	}
 

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

@@ -13,6 +13,7 @@ import redis.clients.jedis.Jedis;
 import redis.clients.jedis.JedisSentinelPool;
 import redis.clients.jedis.Transaction;
 import redis.clients.jedis.exceptions.JedisConnectionException;
+import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.tests.utils.JedisSentinelTestUtil;
 
 public class JedisSentinelPoolTest extends JedisTestBase {
@@ -42,30 +43,24 @@ public class JedisSentinelPoolTest extends JedisTestBase {
 	sentinelJedis2 = new Jedis(sentinel2.getHost(), sentinel2.getPort());
     }
     
-    @Test
+    @Test(expected=JedisConnectionException.class)
-    public void errorMasterNameNotThrowException() throws InterruptedException {
+    public void initializeWithNotAvailableSentinelsShouldThrowException() {
+	Set<String> wrongSentinels = new HashSet<String>();
+	wrongSentinels.add(new HostAndPort("localhost", 65432).toString());
+	wrongSentinels.add(new HostAndPort("localhost", 65431).toString());
+	
+	JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, wrongSentinels);
+	pool.destroy();
+    }
+    
+    @Test(expected=JedisException.class)
+    public void initializeWithNotMonitoredMasterNameShouldThrowException() {
 	final String wrongMasterName = "wrongMasterName";
-	new Thread(new Runnable() {
-	    @Override
-	    public void run() {
-		try {
-		    TimeUnit.SECONDS.sleep(3);
-		    sentinelJedis1.sentinelMonitor(wrongMasterName,
-			    "127.0.0.1", master.getPort(), 2);
-		} catch (InterruptedException e) {
-		    e.printStackTrace();
-		}
-
-	    }
-	}).start();
-
 	JedisSentinelPool pool = new JedisSentinelPool(wrongMasterName,
 		sentinels);
 	pool.destroy();
-	sentinelJedis1.sentinelRemove(wrongMasterName);
     }
     
-    
     @Test
     public void checkCloseableConnections() throws Exception {
 	GenericObjectPoolConfig config = new GenericObjectPoolConfig();