From d22684dfda048a32082afc9621dd04bc37b59ed5 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 29 Nov 2012 17:06:53 +0200 Subject: [PATCH] improve handling of staleness exception during connection init DATAREDIS-109 --- .../connection/jedis/JedisConnection.java | 9 +- .../jedis/JedisConnectionFactory.java | 9 +- .../data/redis/core/RedisTemplate.java | 138 +++++++++--------- 3 files changed, 80 insertions(+), 76 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 e81d49427..a17039288 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 @@ -114,8 +114,15 @@ public class JedisConnection implements RedisConnection { this.dbIndex = dbIndex; // select the db + // if this fail, do manual clean-up before propagating the exception + // as we're inside the constructor if (dbIndex > 0) { - select(dbIndex); + try { + select(dbIndex); + } catch (DataAccessException ex) { + close(); + throw ex; + } } } 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 3474621a9..118b8f86f 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 @@ -21,7 +21,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; -import org.springframework.dao.DataAccessResourceFailureException; +import org.springframework.data.redis.RedisConnectionFailureException; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.util.Assert; @@ -98,7 +98,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, jedis.connect(); return jedis; } catch (Exception ex) { - throw new DataAccessResourceFailureException("Cannot get Jedis connection", ex); + throw new RedisConnectionFailureException("Cannot get Jedis connection", ex); } } @@ -146,11 +146,10 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, public JedisConnection getConnection() { Jedis jedis = fetchJedisConnector(); - return postProcessConnection((usePool ? new JedisConnection(jedis, pool, dbIndex) : new JedisConnection(jedis, - null, dbIndex))); + return postProcessConnection((usePool ? new JedisConnection(jedis, pool, dbIndex) : new JedisConnection(jedis, null, dbIndex))); } - + public DataAccessException translateExceptionIfPossible(RuntimeException ex) { return JedisUtils.convertJedisAccessException(ex); } diff --git a/src/main/java/org/springframework/data/redis/core/RedisTemplate.java b/src/main/java/org/springframework/data/redis/core/RedisTemplate.java index 5a404083c..1a313c469 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/RedisTemplate.java @@ -87,7 +87,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation public RedisTemplate() { } - + public void afterPropertiesSet() { super.afterPropertiesSet(); boolean defaultUsed = false; @@ -116,7 +116,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation } } - + public T execute(RedisCallback action) { return execute(action, isExposeConnection()); } @@ -147,17 +147,18 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation Assert.notNull(action, "Callback object must not be null"); RedisConnectionFactory factory = getConnectionFactory(); - RedisConnection conn = RedisConnectionUtils.getConnection(factory); - - boolean existingConnection = TransactionSynchronizationManager.hasResource(factory); - preProcessConnection(conn, existingConnection); - - boolean pipelineStatus = conn.isPipelined(); - if (pipeline && !pipelineStatus) { - conn.openPipeline(); - } - + RedisConnection conn = null; try { + conn = RedisConnectionUtils.getConnection(factory); + + boolean existingConnection = TransactionSynchronizationManager.hasResource(factory); + preProcessConnection(conn, existingConnection); + + boolean pipelineStatus = conn.isPipelined(); + if (pipeline && !pipelineStatus) { + conn.openPipeline(); + } + RedisConnection connToExpose = (exposeConnection ? conn : createRedisConnectionProxy(conn)); T result = action.doInRedis(connToExpose); @@ -174,7 +175,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation } - + public T execute(SessionCallback session) { RedisConnectionFactory factory = getConnectionFactory(); // bind connection @@ -407,23 +408,23 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation // // RedisOperations // - + public List exec() { return execute(new RedisCallback>() { - + public List doInRedis(RedisConnection connection) throws DataAccessException { return connection.exec(); } }); } - + public void delete(K key) { final byte[] rawKey = rawKey(key); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.del(rawKey); return null; @@ -431,12 +432,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public void delete(Collection keys) { final byte[][] rawKeys = rawKeys(keys); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.del(rawKeys); return null; @@ -444,45 +445,45 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public Boolean hasKey(K key) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.exists(rawKey); } }, true); } - + public Boolean expire(K key, long timeout, TimeUnit unit) { final byte[] rawKey = rawKey(key); final long rawTimeout = unit.toSeconds(timeout); return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.expire(rawKey, rawTimeout); } }, true); } - + public Boolean expireAt(K key, Date date) { final byte[] rawKey = rawKey(key); final long rawTimeout = date.getTime() / 1000; return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.expireAt(rawKey, rawTimeout); } }, true); } - + public void convertAndSend(String channel, Object message) { Assert.hasText(channel, "a non-empty channel is required"); @@ -490,7 +491,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation final byte[] rawMessage = rawValue(message); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.publish(rawChannel, rawMessage); return null; @@ -503,12 +504,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation // Value operations // - + public Long getExpire(K key) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { - + public Long doInRedis(RedisConnection connection) { return Long.valueOf(connection.ttl(rawKey)); } @@ -516,12 +517,11 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation } @SuppressWarnings("unchecked") - public Set keys(K pattern) { final byte[] rawKey = rawKey(pattern); Set rawKeys = execute(new RedisCallback>() { - + public Set doInRedis(RedisConnection connection) { return connection.keys(rawKey); } @@ -530,34 +530,34 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return SerializationUtils.deserialize(rawKeys, keySerializer); } - + public Boolean persist(K key) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.persist(rawKey); } }, true); } - + public Boolean move(K key, final int dbIndex) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.move(rawKey, dbIndex); } }, true); } - + public K randomKey() { byte[] rawKey = execute(new RedisCallback() { - + public byte[] doInRedis(RedisConnection connection) { return connection.randomKey(); } @@ -566,13 +566,13 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return deserializeKey(rawKey); } - + public void rename(K oldKey, K newKey) { final byte[] rawOldKey = rawKey(oldKey); final byte[] rawNewKey = rawKey(newKey); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.rename(rawOldKey, rawNewKey); return null; @@ -580,35 +580,35 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public Boolean renameIfAbsent(K oldKey, K newKey) { final byte[] rawOldKey = rawKey(oldKey); final byte[] rawNewKey = rawKey(newKey); return execute(new RedisCallback() { - + public Boolean doInRedis(RedisConnection connection) { return connection.renameNX(rawOldKey, rawNewKey); } }, true); } - + public DataType type(K key) { final byte[] rawKey = rawKey(key); return execute(new RedisCallback() { - + public DataType doInRedis(RedisConnection connection) { return connection.type(rawKey); } }, true); } - + public void multi() { execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) throws DataAccessException { connection.multi(); return null; @@ -616,11 +616,11 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public void discard() { execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) throws DataAccessException { connection.discard(); return null; @@ -628,12 +628,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public void watch(K key) { final byte[] rawKey = rawKey(key); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.watch(rawKey); return null; @@ -641,12 +641,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public void watch(Collection keys) { final byte[][] rawKeys = rawKeys(keys); execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) { connection.watch(rawKeys); return null; @@ -654,10 +654,10 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation }, true); } - + public void unwatch() { execute(new RedisCallback() { - + public Object doInRedis(RedisConnection connection) throws DataAccessException { connection.unwatch(); return null; @@ -668,18 +668,17 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation // Sort operations @SuppressWarnings("unchecked") - public List sort(SortQuery query) { return sort(query, valueSerializer); } - + public List sort(SortQuery query, RedisSerializer resultSerializer) { final byte[] rawKey = rawKey(query.getKey()); final SortParameters params = QueryUtils.convertQuery(query, stringSerializer); List vals = execute(new RedisCallback>() { - + public List doInRedis(RedisConnection connection) throws DataAccessException { return connection.sort(rawKey, params); } @@ -689,12 +688,11 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation } @SuppressWarnings("unchecked") - public List sort(SortQuery query, BulkMapper bulkMapper) { return sort(query, bulkMapper, valueSerializer); } - + public List sort(SortQuery query, BulkMapper bulkMapper, RedisSerializer resultSerializer) { List values = sort(query, resultSerializer); @@ -719,26 +717,26 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return result; } - + public Long sort(SortQuery query, K storeKey) { final byte[] rawStoreKey = rawKey(storeKey); final byte[] rawKey = rawKey(query.getKey()); final SortParameters params = QueryUtils.convertQuery(query, stringSerializer); return execute(new RedisCallback() { - + public Long doInRedis(RedisConnection connection) throws DataAccessException { return connection.sort(rawKey, params, rawStoreKey); } }, true); } - + public BoundValueOperations boundValueOps(K key) { return new DefaultBoundValueOperations(key, this); } - + public ValueOperations opsForValue() { if (valueOps == null) { valueOps = new DefaultValueOperations(this); @@ -746,7 +744,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return valueOps; } - + public ListOperations opsForList() { if (listOps == null) { listOps = new DefaultListOperations(this); @@ -754,17 +752,17 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return listOps; } - + public BoundListOperations boundListOps(K key) { return new DefaultBoundListOperations(key, this); } - + public BoundSetOperations boundSetOps(K key) { return new DefaultBoundSetOperations(key, this); } - + public SetOperations opsForSet() { if (setOps == null) { setOps = new DefaultSetOperations(this); @@ -772,12 +770,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return setOps; } - + public BoundZSetOperations boundZSetOps(K key) { return new DefaultBoundZSetOperations(key, this); } - + public ZSetOperations opsForZSet() { if (zSetOps == null) { zSetOps = new DefaultZSetOperations(this); @@ -785,12 +783,12 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return zSetOps; } - + public BoundHashOperations boundHashOps(K key) { return new DefaultBoundHashOperations(key, this); } - + public HashOperations opsForHash() { return new DefaultHashOperations(this); }