From aa4d784df00129a57803b39aab507874c102d310 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 2 Nov 2017 15:00:38 +0100 Subject: [PATCH] DATAREDIS-714 - Use Jedis API for database selection. We now rely more on Jedis to select the appropriate Redis database and we no longer select the database when opening/closing a connection. Previously, we always reset the database if a database index greater zero was configured. A properly configured Jedis pool ensures that the appropriate database is selected even if the database was selected during connection interaction. Relying on Jedis reduces the number of issued SELECT commands and improves that way overall performance when executing commands via the Template API. Original Pull Request: #291 --- .../redis/connection/jedis/JedisConnection.java | 16 +++------------- .../connection/jedis/JedisConnectionFactory.java | 4 ++-- .../jedis/JedisConnectionIntegrationTests.java | 16 +++++++++++----- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index 99a2de333..65e65ff04 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java @@ -120,7 +120,7 @@ public class JedisConnection extends AbstractRedisConnection { // select the db // if this fail, do manual clean-up before propagating the exception // as we're inside the constructor - if (dbIndex > 0) { + if (dbIndex != jedis.getDB()) { try { select(dbIndex); } catch (DataAccessException ex) { @@ -281,19 +281,9 @@ public class JedisConnection extends AbstractRedisConnection { if (broken) { pool.returnBrokenResource(jedis); } else { - - // reset the connection - try { - if (dbIndex > 0) { - jedis.select(0); - } - return; - } catch (Exception ex) { - throw convertJedisAccessException(ex); - } finally { - jedis.close(); - } + jedis.close(); } + return; } // else close the connection normally (doing the try/catch dance) Exception exc = null; diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java index c890feffc..b86dc3869 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java @@ -370,7 +370,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, GenericObjectPoolConfig poolConfig = getPoolConfig() != null ? getPoolConfig() : new JedisPoolConfig(); return new JedisSentinelPool(config.getMaster().getName(), convertToJedisSentinelSet(config.getSentinels()), - poolConfig, getConnectTimeout(), getReadTimeout(), getPassword(), Protocol.DEFAULT_DATABASE, getClientName()); + poolConfig, getConnectTimeout(), getReadTimeout(), getPassword(), getDatabase(), getClientName()); } /** @@ -382,7 +382,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, protected Pool createRedisPool() { return new JedisPool(getPoolConfig(), getHostName(), getPort(), getConnectTimeout(), getReadTimeout(), - getPassword(), Protocol.DEFAULT_DATABASE, getClientName(), isUseSsl(), + getPassword(), getDatabase(), getClientName(), isUseSsl(), clientConfiguration.getSslSocketFactory().orElse(null), // clientConfiguration.getSslParameters().orElse(null), // clientConfiguration.getHostnameVerifier().orElse(null)); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java index b5468c9b7..30e38f1b6 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java @@ -36,6 +36,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.redis.RedisConnectionFailureException; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.AbstractConnectionIntegrationTests; import org.springframework.data.redis.connection.ConnectionUtils; @@ -113,13 +114,18 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati factory2.destroy(); } - @Test(expected = InvalidDataAccessApiUsageException.class) + @Test(expected = RedisConnectionFailureException.class) // DATAREDIS-714 public void testCreateConnectionWithDbFailure() { + JedisConnectionFactory factory2 = new JedisConnectionFactory(); factory2.setDatabase(77); factory2.afterPropertiesSet(); - factory2.getConnection(); - factory2.destroy(); + + try { + factory2.getConnection(); + } finally { + factory2.destroy(); + } } @Test @@ -381,8 +387,8 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati @RequiresRedisSentinel(SentinelsAvailable.ONE_ACTIVE) public void shouldReturnSentinelCommandsWhenWhenActiveSentinelFound() { - ((JedisConnection) byteConnection).setSentinelConfiguration(new RedisSentinelConfiguration().master("mymaster") - .sentinel("127.0.0.1", 26379).sentinel("127.0.0.1", 26380)); + ((JedisConnection) byteConnection).setSentinelConfiguration( + new RedisSentinelConfiguration().master("mymaster").sentinel("127.0.0.1", 26379).sentinel("127.0.0.1", 26380)); assertThat(connection.getSentinelConnection(), notNullValue()); }