Polishing.

Refine unlocking by checking whether the lock was actually applied.

Reduce allocations, refine test assertions to check for concurrency.

See #1686
Original pull request: #2879
This commit is contained in:
Mark Paluch
2024-04-19 08:57:23 +02:00
parent c980407f7d
commit 3ab09fbd57
2 changed files with 61 additions and 24 deletions

View File

@@ -32,6 +32,7 @@ import org.springframework.data.redis.connection.ReactiveRedisConnectionFactory;
import org.springframework.data.redis.connection.ReactiveStringCommands;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.connection.RedisStringCommands;
import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
import org.springframework.data.redis.core.types.Expiration;
import org.springframework.data.redis.util.ByteUtils;
@@ -219,8 +220,10 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
return execute(name, connection -> {
boolean wasLocked = false;
if (isLockingCacheWriter()) {
doLock(name, key, value, connection);
wasLocked = true;
}
try {
@@ -242,7 +245,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
return connection.stringCommands().get(key);
} finally {
if (isLockingCacheWriter()) {
if (isLockingCacheWriter() && wasLocked) {
doUnlock(name, connection);
}
}
@@ -319,15 +322,17 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
execute(name, connection -> doLock(name, name, null, connection));
}
@Nullable
protected Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
RedisConnection connection) {
boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue, RedisConnection connection) {
RedisStringCommands commands = connection.stringCommands();
Expiration expiration = Expiration.from(this.lockTtl.getTimeToLive(contextualKey, contextualValue));
byte[] cacheLockKey = createCacheLockKey(name);
while (!ObjectUtils.nullSafeEquals(connection.stringCommands().set(createCacheLockKey(name), new byte[0], expiration, SetOption.SET_IF_ABSENT),true)) {
while (!ObjectUtils.nullSafeEquals(commands.set(cacheLockKey, new byte[0], expiration, SetOption.SET_IF_ABSENT),
true)) {
checkAndPotentiallyWaitUntilUnlocked(name, connection);
}
return true;
}
@@ -341,7 +346,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
}
@Nullable
private Long doUnlock(String name, RedisConnection connection) {
Long doUnlock(String name, RedisConnection connection) {
return connection.keyCommands().del(createCacheLockKey(name));
}
@@ -489,8 +494,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
Mono<?> cacheLockCheck = isLockingCacheWriter() ? waitForLock(connection, name) : Mono.empty();
ReactiveStringCommands stringCommands = connection.stringCommands();
Mono<ByteBuffer> get = shouldExpireWithin(ttl)
? stringCommands.getEx(wrappedKey, Expiration.from(ttl))
Mono<ByteBuffer> get = shouldExpireWithin(ttl) ? stringCommands.getEx(wrappedKey, Expiration.from(ttl))
: stringCommands.get(wrappedKey);
return cacheLockCheck.then(get).map(ByteUtils::getBytes).toFuture();
@@ -502,8 +506,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
return doWithConnection(connection -> {
Mono<?> mono = isLockingCacheWriter()
? doStoreWithLocking(name, key, value, ttl, connection)
Mono<?> mono = isLockingCacheWriter() ? doStoreWithLocking(name, key, value, ttl, connection)
: doStore(key, value, ttl, connection);
return mono.then().toFuture();
@@ -531,7 +534,6 @@ class DefaultRedisCacheWriter implements RedisCacheWriter {
}
}
private Mono<Object> doLock(String name, Object contextualKey, @Nullable Object contextualValue,
ReactiveRedisConnection connection) {