From 99b18c4ca596f7c3d1b3c9a46a435e1697f5024f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 2 Nov 2017 15:00:38 +0100 Subject: [PATCH] DATAREDIS-790 - 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. Original ticket: DATAREDIS-714. --- .../connection/jedis/JedisConnection.java | 18 ++++------------- .../jedis/JedisConnectionFactory.java | 4 ++-- .../JedisConnectionIntegrationTests.java | 20 ++++++++++++------- 3 files changed, 19 insertions(+), 23 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 e684aaf29..3dfe8a5e9 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 @@ -1,5 +1,5 @@ /* - * Copyright 2011-2017 the original author or authors. + * Copyright 2011-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -221,7 +221,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) { @@ -292,19 +292,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 ffde590f4..504129cde 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 @@ -260,7 +260,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, protected Pool createRedisSentinelPool(RedisSentinelConfiguration config) { return new JedisSentinelPool(config.getMaster().getName(), convertToJedisSentinelSet(config.getSentinels()), getPoolConfig() != null ? getPoolConfig() : new JedisPoolConfig(), getTimeoutFrom(getShardInfo()), - getShardInfo().getPassword(), Protocol.DEFAULT_DATABASE, clientName); + getShardInfo().getPassword(), getDatabase(), clientName); } /** @@ -272,7 +272,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, protected Pool createRedisPool() { return new JedisPool(getPoolConfig(), getShardInfo().getHost(), getShardInfo().getPort(), - getTimeoutFrom(getShardInfo()), getShardInfo().getPassword(), Protocol.DEFAULT_DATABASE, clientName, useSsl); + getTimeoutFrom(getShardInfo()), getShardInfo().getPassword(), getDatabase(), clientName, useSsl); } private JedisCluster createCluster() { 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 9253d3888..4483d4bef 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 @@ -1,5 +1,5 @@ /* - * Copyright 2011-2017 the original author or authors. + * Copyright 2011-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,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; @@ -55,7 +56,7 @@ import redis.clients.jedis.JedisPoolConfig; /** * Integration test of {@link JedisConnection} - * + * * @author Costin Leau * @author Jennifer Hickey * @author Thomas Darimont @@ -113,13 +114,18 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati factory2.destroy(); } - @Test(expected = InvalidDataAccessApiUsageException.class) + @Test(expected = RedisConnectionFailureException.class) // DATAREDIS-714, DATAREDIS-790 public void testCreateConnectionWithDbFailure() { + JedisConnectionFactory factory2 = new JedisConnectionFactory(); factory2.setDatabase(77); factory2.afterPropertiesSet(); - factory2.getConnection(); - factory2.destroy(); + + try { + factory2.getConnection(); + } finally { + factory2.destroy(); + } } @Test @@ -385,8 +391,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()); }