Skip to content

Commit b699840

Browse files
authored
GH-11044: Revise cache in RedisLockRegistry
- Use a DefaultLockRegistry to manage local lock instances. - Document the internal registry changes Fixes: #11044 **Auto-cherry-pick to `7.0.x` & `6.5.x`**
1 parent ea9fa2f commit b699840

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
@@ -60,6 +60,7 @@
6060
import org.springframework.data.redis.listener.PatternTopic;
6161
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
6262
import org.springframework.data.redis.listener.Topic;
63+
import org.springframework.integration.support.locks.DefaultLockRegistry;
6364
import org.springframework.integration.support.locks.DistributedLock;
6465
import org.springframework.integration.support.locks.ExpirableLockRegistry;
6566
import org.springframework.integration.support.locks.RenewableLockRegistry;
@@ -103,6 +104,7 @@
103104
* @author Michal Domagala
104105
* @author Severin Kistler
105106
* @author Yordan Tsintsov
107+
* @author Glenn Renfro
106108
*
107109
* @since 4.0
108110
*
@@ -114,14 +116,16 @@ public final class RedisLockRegistry
114116

115117
private static final long DEFAULT_EXPIRE_AFTER = 60000L;
116118

117-
private static final int DEFAULT_CAPACITY = 100_000;
119+
private static final int DEFAULT_CAPACITY = 256;
118120

119121
private static final int DEFAULT_IDLE = 100;
120122

121123
private final Lock lock = new ReentrantLock();
122124

123125
private Duration idleBetweenTries = Duration.ofMillis(DEFAULT_IDLE);
124126

127+
private DefaultLockRegistry defaultLockRegistry = new DefaultLockRegistry();
128+
125129
private final Map<String, RedisLock> locks =
126130
new LinkedHashMap<>(16, 0.75F, true) {
127131

@@ -252,11 +256,14 @@ public void setRenewalTaskScheduler(TaskScheduler renewalTaskScheduler) {
252256

253257
/**
254258
* Set the capacity of cached locks.
255-
* @param cacheCapacity The capacity of cached lock, (default 100_000).
259+
* @param cacheCapacity The capacity of cached lock, (default 256 locks).
256260
* @since 5.5.6
257261
*/
258262
public void setCacheCapacity(int cacheCapacity) {
259263
this.cacheCapacity = cacheCapacity;
264+
// Find the highest power of 2 for (n + 1), then subtract 1 to get the mask
265+
int mask = Integer.highestOneBit(cacheCapacity + 1) - 1;
266+
this.defaultLockRegistry = new DefaultLockRegistry(mask);
260267
}
261268

262269
/**
@@ -421,14 +428,15 @@ private abstract class RedisLock implements DistributedLock {
421428

422429
protected final BoundValueOperations<String, String> boundValueOps;
423430

424-
private final ReentrantLock localLock = new ReentrantLock();
431+
private final ReentrantLock localLock;
425432

426433
private volatile long lockedAt;
427434

428435
private volatile @Nullable ScheduledFuture<?> renewFuture;
429436

430437
private RedisLock(String path) {
431438
this.lockKey = constructLockKey(path);
439+
this.localLock = (ReentrantLock) RedisLockRegistry.this.defaultLockRegistry.obtain(this.lockKey);
432440
this.boundValueOps = RedisLockRegistry.this.redisTemplate.boundValueOps(this.lockKey);
433441
}
434442

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
@@ -1070,6 +1070,50 @@ void testUnlockDoesNotDeleteOtherClientsLock() throws Exception {
10701070
}
10711071
}
10721072

1073+
@Test
1074+
void noSecondLockOnEviction() throws InterruptedException {
1075+
RedisLockRegistry registry = new RedisLockRegistry(redisConnectionFactory, this.registryKey);
1076+
registry.setRedisLockType(testRedisLockType);
1077+
registry.setCacheCapacity(2);
1078+
1079+
CountDownLatch lock1Latch = new CountDownLatch(1);
1080+
CountDownLatch furtherLocksLatch = new CountDownLatch(1);
1081+
1082+
Executors.newSingleThreadExecutor()
1083+
.execute(() -> {
1084+
DistributedLock lock1 = registry.obtain("lock1");
1085+
lock1.lock();
1086+
try {
1087+
furtherLocksLatch.countDown();
1088+
lock1Latch.await(10, TimeUnit.SECONDS);
1089+
}
1090+
catch (InterruptedException e) {
1091+
Thread.currentThread().interrupt();
1092+
}
1093+
finally {
1094+
lock1.unlock();
1095+
}
1096+
});
1097+
1098+
assertThat(furtherLocksLatch.await(10, TimeUnit.SECONDS)).isTrue();
1099+
// Two new locks to trigger cache eviction for the 'lock1'
1100+
registry.obtain("lock2");
1101+
registry.obtain("lock3");
1102+
1103+
// Request 'lock1' again: will trigger new RedisLock instance
1104+
DistributedLock lock1 = registry.obtain("lock1");
1105+
1106+
// Cannot lock because 'lock1' is still locked by another thread
1107+
assertThat(lock1.tryLock(1, TimeUnit.SECONDS)).isFalse();
1108+
1109+
lock1Latch.countDown();
1110+
1111+
assertThatNoException().isThrownBy(() -> {
1112+
lock1.lock();
1113+
lock1.unlock();
1114+
});
1115+
}
1116+
10731117
private Long getExpire(RedisLockRegistry registry, String lockKey) {
10741118
StringRedisTemplate template = createTemplate();
10751119
String registryKey = TestUtils.getPropertyValue(registry, "registryKey");

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

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

14651465
Starting with version 5.5.6, the `RedisLockRegistry` is support automatically clean up cache for redisLocks in `RedisLockRegistry.locks` via `RedisLockRegistry.setCacheCapacity()`.
1466+
The default value is `256`.
14661467
See its JavaDocs for more information.
1468+
This capacity number is also propagated into the `DefaultLockRegistry` used internally in the `RedisLockRegistry` for a pool of shared `ReentrantLock` instances.
1469+
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)`).
1470+
14671471

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

@@ -1507,4 +1511,4 @@ RedisLockRegistry lockRegistry = new RedisLockRegistry("my-lock-key{choose_your_
15071511
lockRegistry.lock();
15081512
# critical section
15091513
lockRegistry.unlock();
1510-
----
1514+
----

0 commit comments

Comments
 (0)