From 2815da6d82b46fa54a4eb6af1a3a12c290d08a00 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 600eca444..63146a109 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 @@ -161,7 +161,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) { @@ -332,19 +332,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 7ae605382..c60882a7e 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 118341e93..bb4b40153 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()); }