GH-3434: Optimize unlinkAvailable in Redis

Fixes https://github.com/spring-projects/spring-integration/issues/3434

Each `RedisLock` has their own `unlinkAvailable` property
therefore we try to `unlink` on every unlock call and also log a WARN
about unavailability of this command in Redis, plus exception

* Move `unlinkAvailable` property to the `RedisLockRegistry` level
to have a check only once on a first `unlock()` call
* Move the whole exception logging onto the DEBUG level, leaving the WARN
only with the `ex.getMessage()`
* Optimize logging the same way in the `RedisMessageStore`

Cherry-pick to `5.3.x & 5.2.x`
This commit is contained in:
Artem Bilan
2020-12-01 14:18:36 -05:00
committed by Gary Russell
parent 97e1692a36
commit 4909f030a1
2 changed files with 35 additions and 25 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2007-2019 the original author or authors.
* Copyright 2007-2020 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.
@@ -139,9 +139,7 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B
this.redisTemplate.unlink(id);
}
catch (Exception ex) {
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
this.unlinkAvailable = false;
unlinkUnavailable(ex);
this.redisTemplate.delete(id);
}
}
@@ -152,6 +150,18 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B
return removedObject;
}
private void unlinkUnavailable(Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
}
else {
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command: " + ex.getMessage());
}
this.unlinkAvailable = false;
}
@Override
protected void doRemoveAll(Collection<Object> ids) {
if (this.unlinkAvailable) {
@@ -159,9 +169,7 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B
this.redisTemplate.unlink(ids);
}
catch (Exception ex) {
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
this.unlinkAvailable = false;
unlinkUnavailable(ex);
this.redisTemplate.delete(ids);
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2019 the original author or authors.
* Copyright 2014-2020 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.
@@ -19,7 +19,6 @@ package org.springframework.integration.redis.util;
import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.Date;
import java.util.Iterator;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
@@ -116,6 +115,8 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
*/
private boolean executorExplicitlySet;
private volatile boolean unlinkAvailable = true;
/**
* Constructs a lock registry with the default (60 second) lock expiration.
* @param connectionFactory The connection factory.
@@ -160,15 +161,12 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
@Override
public void expireUnusedOlderThan(long age) {
Iterator<Map.Entry<String, RedisLock>> iterator = this.locks.entrySet().iterator();
long now = System.currentTimeMillis();
while (iterator.hasNext()) {
Map.Entry<String, RedisLock> entry = iterator.next();
RedisLock lock = entry.getValue();
if (now - lock.getLockedAt() > age && !lock.isAcquiredInThisProcess()) {
iterator.remove();
}
}
this.locks.entrySet()
.removeIf((entry) -> {
RedisLock lock = entry.getValue();
return now - lock.getLockedAt() > age && !lock.isAcquiredInThisProcess();
});
}
@Override
@@ -184,8 +182,6 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
private final ReentrantLock localLock = new ReentrantLock();
private volatile boolean unlinkAvailable = true;
private volatile long lockedAt;
private RedisLock(String path) {
@@ -193,7 +189,7 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
}
private String constructLockKey(String path) {
return RedisLockRegistry.this.registryKey + ":" + path;
return RedisLockRegistry.this.registryKey + ':' + path;
}
public long getLockedAt() {
@@ -331,14 +327,20 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
}
private void removeLockKey() {
if (this.unlinkAvailable) {
if (RedisLockRegistry.this.unlinkAvailable) {
try {
RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
}
catch (Exception ex) {
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
this.unlinkAvailable = false;
RedisLockRegistry.this.unlinkAvailable = false;
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
}
else {
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command: " + ex.getMessage());
}
RedisLockRegistry.this.redisTemplate.delete(this.lockKey);
}
}
@@ -359,7 +361,7 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
@Override
public String toString() {
SimpleDateFormat dateFormat = new SimpleDateFormat("YYYY-MM-dd@HH:mm:ss.SSS");
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd@HH:mm:ss.SSS");
return "RedisLock [lockKey=" + this.lockKey
+ ",lockedAt=" + dateFormat.format(new Date(this.lockedAt))
+ ", clientId=" + RedisLockRegistry.this.clientId