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
This commit is contained in:
Mark Paluch
2022-07-14 16:04:37 +02:00
parent ed816393f2
commit c600d0e22d
2 changed files with 34 additions and 18 deletions

View File

@@ -19,10 +19,12 @@ import java.io.IOException;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.dao.DataAccessException;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.dao.InvalidDataAccessResourceUsageException;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
@@ -33,6 +35,8 @@ import org.springframework.util.Assert;
*/
public abstract class AbstractRedisConnection implements RedisConnection {
private final Log LOGGER = LogFactory.getLog(getClass());
private @Nullable RedisSentinelConfiguration sentinelConfiguration;
private final Map<RedisNode, RedisSentinelConnection> connectionCache = new ConcurrentHashMap<>();
@@ -96,18 +100,23 @@ public abstract class AbstractRedisConnection implements RedisConnection {
@Override
public void close() throws DataAccessException {
if (!connectionCache.isEmpty()) {
for (RedisNode node : connectionCache.keySet()) {
RedisSentinelConnection connection = connectionCache.remove(node);
if (connection.isOpen()) {
try {
connection.close();
} catch (IOException e) {
throw new RedisSystemException("Failed to close sentinel connection", e);
}
}
if (connectionCache.isEmpty()) {
return;
}
for (RedisNode node : connectionCache.keySet()) {
RedisSentinelConnection connection = connectionCache.remove(node);
if (!connection.isOpen()) {
continue;
}
try {
connection.close();
} catch (IOException e) {
LOGGER.info("Failed to close sentinel connection", e);
}
}
}
}

View File

@@ -55,6 +55,9 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Supplier;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeanUtils;
import org.springframework.core.convert.converter.Converter;
import org.springframework.dao.DataAccessException;
@@ -90,6 +93,8 @@ import org.springframework.util.ObjectUtils;
*/
public class LettuceConnection extends AbstractRedisConnection {
private final Log LOGGER = LogFactory.getLog(getClass());
static final RedisCodec<byte[], byte[]> CODEC = ByteArrayCodec.INSTANCE;
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy(
@@ -342,7 +347,7 @@ public class LettuceConnection extends AbstractRedisConnection {
}
@Override
public void close() throws DataAccessException {
public void close() {
super.close();
@@ -352,7 +357,11 @@ public class LettuceConnection extends AbstractRedisConnection {
isClosed = true;
reset();
try {
reset();
} catch (RuntimeException e) {
LOGGER.debug("Failed to reset connection during close", e);
}
}
private void reset() {
@@ -629,8 +638,7 @@ public class LettuceConnection extends AbstractRedisConnection {
checkSubscription();
if (isQueueing() || isPipelined()) {
throw new InvalidDataAccessApiUsageException(
"Transaction/Pipelining is not supported for Pub/Sub subscriptions");
throw new InvalidDataAccessApiUsageException("Transaction/Pipelining is not supported for Pub/Sub subscriptions");
}
try {
@@ -647,8 +655,7 @@ public class LettuceConnection extends AbstractRedisConnection {
checkSubscription();
if (isQueueing() || isPipelined()) {
throw new InvalidDataAccessApiUsageException(
"Transaction/Pipelining is not supported for Pub/Sub subscriptions");
throw new InvalidDataAccessApiUsageException("Transaction/Pipelining is not supported for Pub/Sub subscriptions");
}
try {