Skip to content

Commit 0c51d99

Browse files
committed
Polishing.
We now ensure proper exception handling in connection close methods to avoid resource leaks. Also, exceptions during connection close are no longer thrown to ensure proper resource cleanup behavior and API design. See #2356
1 parent e133703 commit 0c51d99

File tree

2 files changed

+32
-14
lines changed

2 files changed

+32
-14
lines changed

src/main/java/org/springframework/data/redis/connection/AbstractRedisConnection.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import java.util.Map;
2020
import java.util.concurrent.ConcurrentHashMap;
2121

22+
import org.apache.commons.logging.Log;
23+
import org.apache.commons.logging.LogFactory;
24+
2225
import org.springframework.dao.DataAccessException;
2326
import org.springframework.dao.InvalidDataAccessApiUsageException;
2427
import org.springframework.dao.InvalidDataAccessResourceUsageException;
25-
import org.springframework.data.redis.RedisSystemException;
2628
import org.springframework.lang.Nullable;
2729
import org.springframework.util.Assert;
2830

@@ -33,6 +35,8 @@
3335
*/
3436
public abstract class AbstractRedisConnection implements DefaultedRedisConnection {
3537

38+
private final Log LOGGER = LogFactory.getLog(getClass());
39+
3640
private @Nullable RedisSentinelConfiguration sentinelConfiguration;
3741
private final Map<RedisNode, RedisSentinelConnection> connectionCache = new ConcurrentHashMap<>();
3842

@@ -104,18 +108,23 @@ protected RedisSentinelConnection getSentinelConnection(RedisNode sentinel) {
104108
@Override
105109
public void close() throws DataAccessException {
106110

107-
if (!connectionCache.isEmpty()) {
108-
for (RedisNode node : connectionCache.keySet()) {
109-
RedisSentinelConnection connection = connectionCache.remove(node);
110-
if (connection.isOpen()) {
111-
try {
112-
connection.close();
113-
} catch (IOException e) {
114-
throw new RedisSystemException("Failed to close sentinel connection", e);
115-
}
116-
}
111+
if (connectionCache.isEmpty()) {
112+
return;
113+
}
114+
115+
for (RedisNode node : connectionCache.keySet()) {
116+
117+
RedisSentinelConnection connection = connectionCache.remove(node);
118+
119+
if (!connection.isOpen()) {
120+
continue;
121+
}
122+
123+
try {
124+
connection.close();
125+
} catch (IOException e) {
126+
LOGGER.info("Failed to close sentinel connection", e);
117127
}
118128
}
119129
}
120-
121130
}

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@
5656
import java.util.concurrent.atomic.AtomicLong;
5757
import java.util.function.Supplier;
5858

59+
import org.apache.commons.logging.Log;
60+
import org.apache.commons.logging.LogFactory;
61+
5962
import org.springframework.beans.BeanUtils;
6063
import org.springframework.core.convert.converter.Converter;
6164
import org.springframework.dao.DataAccessException;
@@ -91,6 +94,8 @@
9194
*/
9295
public class LettuceConnection extends AbstractRedisConnection {
9396

97+
private final Log LOGGER = LogFactory.getLog(getClass());
98+
9499
static final RedisCodec<byte[], byte[]> CODEC = ByteArrayCodec.INSTANCE;
95100

96101
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy(
@@ -414,7 +419,7 @@ public Object execute(String command, @Nullable CommandOutput commandOutputTypeH
414419
* @see org.springframework.data.redis.connection.AbstractRedisConnection#close()
415420
*/
416421
@Override
417-
public void close() throws DataAccessException {
422+
public void close() {
418423

419424
super.close();
420425

@@ -424,7 +429,11 @@ public void close() throws DataAccessException {
424429

425430
isClosed = true;
426431

427-
reset();
432+
try {
433+
reset();
434+
} catch (RuntimeException e) {
435+
LOGGER.debug("Failed to reset connection during close", e);
436+
}
428437
}
429438

430439
private void reset() {

0 commit comments

Comments
 (0)