diff --git a/src/main/java/org/springframework/data/redis/core/RedisConnectionUtils.java b/src/main/java/org/springframework/data/redis/core/RedisConnectionUtils.java index b3db62fd4..7fcf5f0a8 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisConnectionUtils.java +++ b/src/main/java/org/springframework/data/redis/core/RedisConnectionUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -154,11 +154,12 @@ public abstract class RedisConnectionUtils { if (!connHolder.isTransactionSyncronisationActive()) { connHolder.setTransactionSyncronisationActive(true); - + RedisConnection conn = connHolder.getConnection(); conn.multi(); - TransactionSynchronizationManager.registerSynchronization(new RedisTransactionSynchronizer(connHolder, conn)); + TransactionSynchronizationManager.registerSynchronization(new RedisTransactionSynchronizer(connHolder, conn, + factory)); } } } @@ -252,26 +253,31 @@ public abstract class RedisConnectionUtils { } /** - * A {@link TransactionSynchronizationAdapter} that makes sure that the associated RedisConnection is released after the transaction completes. + * A {@link TransactionSynchronizationAdapter} that makes sure that the associated RedisConnection is released after + * the transaction completes. * * @author Christoph Strobl * @author Thomas Darimont */ private static class RedisTransactionSynchronizer extends TransactionSynchronizationAdapter { - + private final RedisConnectionHolder connHolder; private final RedisConnection connection; + private final RedisConnectionFactory factory; /** * Creates a new {@link RedisTransactionSynchronizer}. * * @param connHolder * @param connection + * @param factory */ - private RedisTransactionSynchronizer(RedisConnectionHolder connHolder, RedisConnection connection) { - + private RedisTransactionSynchronizer(RedisConnectionHolder connHolder, RedisConnection connection, + RedisConnectionFactory factory) { + this.connHolder = connHolder; this.connection = connection; + this.factory = factory; } @Override @@ -294,9 +300,10 @@ public abstract class RedisConnectionUtils { if (log.isDebugEnabled()) { log.debug("Closing bound connection after transaction completed with " + status); } - + connHolder.setTransactionSyncronisationActive(false); connection.close(); + TransactionSynchronizationManager.unbindResource(factory); } } } 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 fe7673a51..70d7e2bfc 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/RedisTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-2015 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. @@ -198,9 +198,7 @@ public class RedisTemplate extends RedisAccessor implements RedisOperation return postProcessResult(result, connToUse, existingConnection); } finally { - if (enableTransactionSupport) { - RedisConnectionUtils.unbindConnection(factory); - } else { + if (!enableTransactionSupport) { RedisConnectionUtils.releaseConnection(conn, factory); } } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisTransactionalConnectionStarvationTest.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisTransactionalConnectionStarvationTest.java new file mode 100644 index 000000000..20a6f5583 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisTransactionalConnectionStarvationTest.java @@ -0,0 +1,98 @@ +/* + * Copyright 2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.connection.jedis; + +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.SettingsUtils; +import org.springframework.data.redis.connection.AbstractTransactionalTestBase; +import org.springframework.data.redis.connection.jedis.JedisTransactionalConnectionStarvationTest.PooledJedisContextConfiguration; +import org.springframework.data.redis.core.StringRedisTemplate; +import org.springframework.data.redis.core.ValueOperations; +import org.springframework.test.annotation.Rollback; +import org.springframework.test.context.ContextConfiguration; + +import redis.clients.jedis.JedisPoolConfig; + +/** + * @author Thomas Darimont + */ +@ContextConfiguration(classes = { PooledJedisContextConfiguration.class }) +public class JedisTransactionalConnectionStarvationTest extends AbstractTransactionalTestBase { + + protected static final int MAX_CONNECTIONS = 5; + + @Autowired StringRedisTemplate template; + + protected void tryOperations(int numOperationsToTry) { + + ValueOperations ops = template.opsForValue(); + + for (int i = 0; i < numOperationsToTry; i++) { + ops.set("test-key-" + i, "test-value-" + i); + } + } + + /** + * @see DATAREDIS-332 + */ + @Test + @Rollback + public void testNumberOfOperationsIsOne() { + tryOperations(1); + } + + /** + * @see DATAREDIS-332 + */ + @Test + @Rollback + public void testNumberOfOperationsEqualToNumberOfConnections() { + tryOperations(MAX_CONNECTIONS); + } + + /** + * @see DATAREDIS-332 + */ + @Test + @Rollback + public void testNumberOfOperationsGreaterThanNumberOfConnections() { + tryOperations(MAX_CONNECTIONS + 1); + } + + @Configuration + public static class PooledJedisContextConfiguration extends RedisContextConfiguration { + + @Bean + public JedisConnectionFactory redisConnectionFactory() { + + JedisConnectionFactory connectionFactory = new JedisConnectionFactory(); + connectionFactory.setHostName(SettingsUtils.getHost()); + connectionFactory.setPort(SettingsUtils.getPort()); + + JedisPoolConfig poolConfig = new JedisPoolConfig(); + poolConfig.setMaxTotal(MAX_CONNECTIONS); + poolConfig.setMaxIdle(MAX_CONNECTIONS); + poolConfig.setMaxWaitMillis(2000L); + connectionFactory.setPoolConfig(poolConfig); + connectionFactory.afterPropertiesSet(); + + return connectionFactory; + } + } +}