Skip to content

Commit c7ce9da

Browse files
artembilangaryrussell
authored andcommitted
GH-3434: Optimize unlinkAvailable in Redis
Fixes #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`
1 parent 106658f commit c7ce9da

File tree

2 files changed

+35
-25
lines changed

2 files changed

+35
-25
lines changed

spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java

+15-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2007-2019 the original author or authors.
2+
* Copyright 2007-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -139,9 +139,7 @@ protected Object doRemove(Object id) {
139139
this.redisTemplate.unlink(id);
140140
}
141141
catch (Exception ex) {
142-
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
143-
"falling back to the regular DELETE command", ex);
144-
this.unlinkAvailable = false;
142+
unlinkUnavailable(ex);
145143
this.redisTemplate.delete(id);
146144
}
147145
}
@@ -152,16 +150,26 @@ protected Object doRemove(Object id) {
152150
return removedObject;
153151
}
154152

153+
private void unlinkUnavailable(Exception ex) {
154+
if (logger.isDebugEnabled()) {
155+
logger.debug("The UNLINK command has failed (not supported on the Redis server?); " +
156+
"falling back to the regular DELETE command", ex);
157+
}
158+
else {
159+
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
160+
"falling back to the regular DELETE command: " + ex.getMessage());
161+
}
162+
this.unlinkAvailable = false;
163+
}
164+
155165
@Override
156166
protected void doRemoveAll(Collection<Object> ids) {
157167
if (this.unlinkAvailable) {
158168
try {
159169
this.redisTemplate.unlink(ids);
160170
}
161171
catch (Exception ex) {
162-
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
163-
"falling back to the regular DELETE command", ex);
164-
this.unlinkAvailable = false;
172+
unlinkUnavailable(ex);
165173
this.redisTemplate.delete(ids);
166174
}
167175
}

spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java

+20-18
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2019 the original author or authors.
2+
* Copyright 2014-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@
1919
import java.text.SimpleDateFormat;
2020
import java.util.Collections;
2121
import java.util.Date;
22-
import java.util.Iterator;
2322
import java.util.Map;
2423
import java.util.UUID;
2524
import java.util.concurrent.ConcurrentHashMap;
@@ -116,6 +115,8 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
116115
*/
117116
private boolean executorExplicitlySet;
118117

118+
private volatile boolean unlinkAvailable = true;
119+
119120
/**
120121
* Constructs a lock registry with the default (60 second) lock expiration.
121122
* @param connectionFactory The connection factory.
@@ -160,15 +161,12 @@ public Lock obtain(Object lockKey) {
160161

161162
@Override
162163
public void expireUnusedOlderThan(long age) {
163-
Iterator<Map.Entry<String, RedisLock>> iterator = this.locks.entrySet().iterator();
164164
long now = System.currentTimeMillis();
165-
while (iterator.hasNext()) {
166-
Map.Entry<String, RedisLock> entry = iterator.next();
167-
RedisLock lock = entry.getValue();
168-
if (now - lock.getLockedAt() > age && !lock.isAcquiredInThisProcess()) {
169-
iterator.remove();
170-
}
171-
}
165+
this.locks.entrySet()
166+
.removeIf((entry) -> {
167+
RedisLock lock = entry.getValue();
168+
return now - lock.getLockedAt() > age && !lock.isAcquiredInThisProcess();
169+
});
172170
}
173171

174172
@Override
@@ -184,16 +182,14 @@ private final class RedisLock implements Lock {
184182

185183
private final ReentrantLock localLock = new ReentrantLock();
186184

187-
private volatile boolean unlinkAvailable = true;
188-
189185
private volatile long lockedAt;
190186

191187
private RedisLock(String path) {
192188
this.lockKey = constructLockKey(path);
193189
}
194190

195191
private String constructLockKey(String path) {
196-
return RedisLockRegistry.this.registryKey + ":" + path;
192+
return RedisLockRegistry.this.registryKey + ':' + path;
197193
}
198194

199195
public long getLockedAt() {
@@ -331,14 +327,20 @@ public void unlock() {
331327
}
332328

333329
private void removeLockKey() {
334-
if (this.unlinkAvailable) {
330+
if (RedisLockRegistry.this.unlinkAvailable) {
335331
try {
336332
RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
337333
}
338334
catch (Exception ex) {
339-
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
340-
"falling back to the regular DELETE command", ex);
341-
this.unlinkAvailable = false;
335+
RedisLockRegistry.this.unlinkAvailable = false;
336+
if (LOGGER.isDebugEnabled()) {
337+
LOGGER.debug("The UNLINK command has failed (not supported on the Redis server?); " +
338+
"falling back to the regular DELETE command", ex);
339+
}
340+
else {
341+
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
342+
"falling back to the regular DELETE command: " + ex.getMessage());
343+
}
342344
RedisLockRegistry.this.redisTemplate.delete(this.lockKey);
343345
}
344346
}
@@ -359,7 +361,7 @@ public boolean isAcquiredInThisProcess() {
359361

360362
@Override
361363
public String toString() {
362-
SimpleDateFormat dateFormat = new SimpleDateFormat("YYYY-MM-dd@HH:mm:ss.SSS");
364+
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd@HH:mm:ss.SSS");
363365
return "RedisLock [lockKey=" + this.lockKey
364366
+ ",lockedAt=" + dateFormat.format(new Date(this.lockedAt))
365367
+ ", clientId=" + RedisLockRegistry.this.clientId

0 commit comments

Comments
 (0)