From c5336310bd4e0ea3879ef961c21fa69e098699f1 Mon Sep 17 00:00:00 2001 From: Jungtaek Lim Date: Sat, 27 Sep 2014 14:09:13 +0900 Subject: [PATCH 1/2] Fail fast while initializing JedisSentinelPool * wrong master name is specified * all sentinel is down or not reached --- .../clients/jedis/JedisSentinelPool.java | 74 ++++++++++--------- .../jedis/tests/JedisSentinelPoolTest.java | 31 ++++---- 2 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java index b3e6abd..443958d 100644 --- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java +++ b/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 { @@ -112,52 +113,55 @@ public class JedisSentinelPool extends Pool { 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 - .split(":"))); + Jedis jedis = null; + try { + jedis = new Jedis(hap.getHost(), hap.getPort()); - log.fine("Connecting to Sentinel " + hap); + if (master == null) { + List masterAddr = jedis + .sentinelGetMasterAddrByName(masterName); - Jedis jedis = null; - try { - jedis = new Jedis(hap.getHost(), hap.getPort()); + // connected to sentinel... + sentinelAvailable = true; - if (master == null) { - List masterAddr = jedis - .sentinelGetMasterAddrByName(masterName); - if (masterAddr == null || masterAddr.size() != 2) { - log.warning("Can not get master addr, master name: " - + masterName + ". Sentinel: " + hap + "."); - continue; - } - - master = toHostAndPort(masterAddr); - log.fine("Found Redis master at " + master); - break outer; - } - } catch (JedisConnectionException e) { - log.warning("Cannot connect to sentinel running @ " + hap - + ". Trying next one."); - } finally { - if (jedis != null) { - jedis.close(); + if (masterAddr == null || masterAddr.size() != 2) { + log.warning("Can not get master addr, master name: " + + masterName + ". Sentinel: " + hap + "."); + continue; } + + master = toHostAndPort(masterAddr); + log.fine("Found Redis master at " + master); + break; + } + } catch (JedisConnectionException e) { + log.warning("Cannot connect to sentinel running @ " + hap + + ". Trying next one."); + } finally { + if (jedis != null) { + jedis.close(); } } + } - try { - log.severe("All sentinels down, cannot determine where is " - + masterName + " master is running... sleeping 1000ms."); - Thread.sleep(1000); - } catch (InterruptedException e) { - e.printStackTrace(); + if (master == null) { + if (sentinelAvailable) { + // can connect to sentinel, but master name seems to not monitored + throw new JedisException("Can connect to sentinel, but " + masterName + + " seems to be not monitored..."); + } else { + throw new JedisConnectionException("All sentinels down, cannot determine where is " + + masterName + " master is running..."); } } diff --git a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java index 1fac249..3db0072 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisSentinelPoolTest.java +++ b/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,29 +43,23 @@ public class JedisSentinelPoolTest extends JedisTestBase { sentinelJedis2 = new Jedis(sentinel2.getHost(), sentinel2.getPort()); } - @Test - public void errorMasterNameNotThrowException() throws InterruptedException { + @Test(expected=JedisConnectionException.class) + public void initializeWithNotAvailableSentinelsShouldThrowException() { + Set wrongSentinels = new HashSet(); + 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 { From 9c296000a25f269345ae15c764821cb1948c789b Mon Sep 17 00:00:00 2001 From: Jungtaek Lim Date: Sun, 5 Oct 2014 10:52:05 +0900 Subject: [PATCH 2/2] Remove unnecessary if state --- .../clients/jedis/JedisSentinelPool.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java index 443958d..6c0bdad 100644 --- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java +++ b/src/main/java/redis/clients/jedis/JedisSentinelPool.java @@ -127,23 +127,21 @@ public class JedisSentinelPool extends Pool { try { jedis = new Jedis(hap.getHost(), hap.getPort()); - if (master == null) { - List masterAddr = jedis - .sentinelGetMasterAddrByName(masterName); + List masterAddr = jedis + .sentinelGetMasterAddrByName(masterName); - // connected to sentinel... - sentinelAvailable = true; + // connected to sentinel... + sentinelAvailable = true; - if (masterAddr == null || masterAddr.size() != 2) { - log.warning("Can not get master addr, master name: " - + masterName + ". Sentinel: " + hap + "."); - continue; - } - - master = toHostAndPort(masterAddr); - log.fine("Found Redis master at " + master); - break; + if (masterAddr == null || masterAddr.size() != 2) { + log.warning("Can not get master addr, master name: " + + masterName + ". Sentinel: " + hap + "."); + continue; } + + master = toHostAndPort(masterAddr); + log.fine("Found Redis master at " + master); + break; } catch (JedisConnectionException e) { log.warning("Cannot connect to sentinel running @ " + hap + ". Trying next one.");