Skip to content

Commit b024f7f

Browse files
cppwfsartembilan
authored andcommitted
GH-11044: Revise cache in RedisLockRegistry
- Use a DefaultLockRegistry to manage local lock instances. - Document the internal registry changes Fixes: #11044
1 parent 202b092 commit b024f7f

3 files changed

Lines changed: 60 additions & 4 deletions

File tree

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import org.springframework.data.redis.listener.PatternTopic;
5757
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
5858
import org.springframework.data.redis.listener.Topic;
59+
import org.springframework.integration.support.locks.DefaultLockRegistry;
5960
import org.springframework.integration.support.locks.ExpirableLockRegistry;
6061
import org.springframework.integration.support.locks.RenewableLockRegistry;
6162
import org.springframework.scheduling.TaskScheduler;
@@ -96,6 +97,7 @@
9697
* @author Alex Peelman
9798
* @author Youbin Wu
9899
* @author Severin Kistler
100+
* @author Glenn Renfro
99101
*
100102
* @since 4.0
101103
*
@@ -106,14 +108,16 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
106108

107109
private static final long DEFAULT_EXPIRE_AFTER = 60000L;
108110

109-
private static final int DEFAULT_CAPACITY = 100_000;
111+
private static final int DEFAULT_CAPACITY = 256;
110112

111113
private static final int DEFAULT_IDLE = 100;
112114

113115
private final Lock lock = new ReentrantLock();
114116

115117
private Duration idleBetweenTries = Duration.ofMillis(DEFAULT_IDLE);
116118

119+
private DefaultLockRegistry defaultLockRegistry = new DefaultLockRegistry();
120+
117121
private final Map<String, RedisLock> locks =
118122
new LinkedHashMap<>(16, 0.75F, true) {
119123

@@ -227,11 +231,14 @@ public void setRenewalTaskScheduler(TaskScheduler renewalTaskScheduler) {
227231

228232
/**
229233
* Set the capacity of cached locks.
230-
* @param cacheCapacity The capacity of cached lock, (default 100_000).
234+
* @param cacheCapacity The capacity of cached lock, (default 256 locks).
231235
* @since 5.5.6
232236
*/
233237
public void setCacheCapacity(int cacheCapacity) {
234238
this.cacheCapacity = cacheCapacity;
239+
// Find the highest power of 2 for (n + 1), then subtract 1 to get the mask
240+
int mask = Integer.highestOneBit(cacheCapacity + 1) - 1;
241+
this.defaultLockRegistry = new DefaultLockRegistry(mask);
235242
}
236243

237244
/**
@@ -382,14 +389,15 @@ private abstract class RedisLock implements Lock {
382389

383390
protected final String lockKey;
384391

385-
private final ReentrantLock localLock = new ReentrantLock();
392+
private final ReentrantLock localLock;
386393

387394
private volatile long lockedAt;
388395

389396
private volatile ScheduledFuture<?> renewFuture;
390397

391398
private RedisLock(String path) {
392399
this.lockKey = constructLockKey(path);
400+
this.localLock = (ReentrantLock) RedisLockRegistry.this.defaultLockRegistry.obtain(this.lockKey);
393401
}
394402

395403
private String constructLockKey(String path) {

spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,50 @@ void testInitialiseWithCustomExecutor() {
951951
assertThatNoException().isThrownBy(() -> redisLockRegistry.setExecutor(mock()));
952952
}
953953

954+
@Test
955+
void noSecondLockOnEviction() throws InterruptedException {
956+
RedisLockRegistry registry = new RedisLockRegistry(redisConnectionFactory, this.registryKey);
957+
registry.setRedisLockType(testRedisLockType);
958+
registry.setCacheCapacity(2);
959+
960+
CountDownLatch lock1Latch = new CountDownLatch(1);
961+
CountDownLatch furtherLocksLatch = new CountDownLatch(1);
962+
963+
Executors.newSingleThreadExecutor()
964+
.execute(() -> {
965+
Lock lock1 = registry.obtain("lock1");
966+
lock1.lock();
967+
try {
968+
furtherLocksLatch.countDown();
969+
lock1Latch.await(10, TimeUnit.SECONDS);
970+
}
971+
catch (InterruptedException e) {
972+
Thread.currentThread().interrupt();
973+
}
974+
finally {
975+
lock1.unlock();
976+
}
977+
});
978+
979+
assertThat(furtherLocksLatch.await(10, TimeUnit.SECONDS)).isTrue();
980+
// Two new locks to trigger cache eviction for the 'lock1'
981+
registry.obtain("lock2");
982+
registry.obtain("lock3");
983+
984+
// Request 'lock1' again: will trigger new RedisLock instance
985+
Lock lock1 = registry.obtain("lock1");
986+
987+
// Cannot lock because 'lock1' is still locked by another thread
988+
assertThat(lock1.tryLock(1, TimeUnit.SECONDS)).isFalse();
989+
990+
lock1Latch.countDown();
991+
992+
assertThatNoException().isThrownBy(() -> {
993+
lock1.lock();
994+
lock1.unlock();
995+
});
996+
}
997+
954998
private Long getExpire(RedisLockRegistry registry, String lockKey) {
955999
StringRedisTemplate template = createTemplate();
9561000
String registryKey = TestUtils.getPropertyValue(registry, "registryKey", String.class);

src/reference/antora/modules/ROOT/pages/redis.adoc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,11 @@ You should set the expiry at a large enough value to prevent this condition, but
847847
Starting with version 5.0, the `RedisLockRegistry` implements `ExpirableLockRegistry`, which removes locks last acquired more than `age` ago and that are not currently locked.
848848

849849
Starting with version 5.5.6, the `RedisLockRegistry` is support automatically clean up cache for redisLocks in `RedisLockRegistry.locks` via `RedisLockRegistry.setCacheCapacity()`.
850+
The default value is `256`.
850851
See its JavaDocs for more information.
852+
This capacity number is also propagated into the `DefaultLockRegistry` used internally in the `RedisLockRegistry` for a pool of shared `ReentrantLock` instances.
853+
So, if higher concurrent access (the number of unique lock keys used in parallel) is expected (or allowed) for the application, the `cacheCapacity` should be increased respectively, with the closest power of 2 in mind (essentially `Integer.highestOneBit(cacheCapacity + 1)`).
854+
851855

852856
Starting with version 5.5.13, the `RedisLockRegistry` exposes a `setRedisLockType(RedisLockType)` option to determine in which mode a Redis lock acquisition should happen:
853857

@@ -884,4 +888,4 @@ RedisLockRegistry lockRegistry = new RedisLockRegistry("my-lock-key{choose_your_
884888
lockRegistry.lock();
885889
# critical section
886890
lockRegistry.unlock();
887-
----
891+
----

0 commit comments

Comments
 (0)