Skip to content

GH-8699: Enhancing unlock() Method of RedisLock with Atomic Redis Ope… #8700

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,12 @@ public long getLockedAt() {
/**
* Unlock the lock using the unlink method in redis.
*/
protected abstract void removeLockKeyInnerUnlink();
protected abstract boolean removeLockKeyInnerUnlink();

/**
* Unlock the lock using the delete method in redis.
*/
protected abstract void removeLockKeyInnerDelete();
protected abstract boolean removeLockKeyInnerDelete();

@Override
public final void lock() {
Expand Down Expand Up @@ -454,11 +454,6 @@ public final void unlock() {
return;
}
try {
if (!isAcquiredInThisProcess()) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}

if (Thread.currentThread().isInterrupted()) {
RedisLockRegistry.this.executor.execute(this::removeLockKey);
}
Expand All @@ -480,8 +475,9 @@ public final void unlock() {

private void removeLockKey() {
if (RedisLockRegistry.this.unlinkAvailable) {
Boolean unlinkResult = null;
try {
removeLockKeyInnerUnlink();
unlinkResult = removeLockKeyInnerUnlink();
return;
}
catch (Exception ex) {
Expand All @@ -494,9 +490,17 @@ private void removeLockKey() {
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command: " + ex.getMessage());
}
} finally {
if (Boolean.FALSE.equals(unlinkResult)) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
}
}
removeLockKeyInnerDelete();
if (!removeLockKeyInnerDelete()) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
}

@Override
Expand Down Expand Up @@ -559,19 +563,23 @@ private RedisLockRegistry getOuterType() {

private final class RedisPubSubLock extends RedisLock {

private static final String UNLINK_UNLOCK_SCRIPT =
"if (redis.call('unlink', KEYS[1]) == 1) then " +
"redis.call('publish', ARGV[1], KEYS[1]) " +
"return true " +
"end " +
"return false";
private static final String UNLINK_UNLOCK_SCRIPT = """
local lockClientId = redis.call('GET', KEYS[1])
if (lockClientId == ARGV[1] and redis.call('UNLINK', KEYS[1]) == 1) then
redis.call('PUBLISH', ARGV[2], KEYS[1])
return true
end
return false
""";

private static final String DELETE_UNLOCK_SCRIPT =
"if (redis.call('del', KEYS[1]) == 1) then " +
"redis.call('publish', ARGV[1], KEYS[1]) " +
"return true " +
"end " +
"return false";
private static final String DELETE_UNLOCK_SCRIPT = """
local lockClientId = redis.call('GET', KEYS[1])
if (lockClientId == ARGV[1] and redis.call('DEL', KEYS[1]) == 1) then
redis.call('PUBLISH', ARGV[2], KEYS[1])
return true
end
return false
""";

private static final RedisScript<Boolean>
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);
Expand All @@ -589,17 +597,17 @@ protected boolean tryRedisLockInner(long time) throws ExecutionException, Interr
}

@Override
protected void removeLockKeyInnerUnlink() {
RedisLockRegistry.this.redisTemplate.execute(
protected boolean removeLockKeyInnerUnlink() {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.unLockChannelKey);
RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey));
}

@Override
protected void removeLockKeyInnerDelete() {
RedisLockRegistry.this.redisTemplate.execute(
protected boolean removeLockKeyInnerDelete() {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.unLockChannelKey);
RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey));

}

Expand Down Expand Up @@ -694,6 +702,30 @@ private void unlockNotify(String lockKey) {

private final class RedisSpinLock extends RedisLock {

private static final String UNLINK_UNLOCK_SCRIPT = """
local lockClientId = redis.call('GET', KEYS[1])
if lockClientId == ARGV[1] then
redis.call('UNLINK', KEYS[1])
return true
end
return false
""";

private static final String DELETE_UNLOCK_SCRIPT = """
local lockClientId = redis.call('GET', KEYS[1])
if lockClientId == ARGV[1] then
redis.call('DEL', KEYS[1])
return true
end
return false
""";

private static final RedisScript<Boolean>
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);

private static final RedisScript<Boolean>
DELETE_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(DELETE_UNLOCK_SCRIPT, Boolean.class);

private RedisSpinLock(String path) {
super(path);
}
Expand All @@ -718,13 +750,17 @@ protected boolean tryRedisLockInner(long time) throws InterruptedException {
}

@Override
protected void removeLockKeyInnerUnlink() {
RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
protected boolean removeLockKeyInnerUnlink() {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.clientId));
}

@Override
protected void removeLockKeyInnerDelete() {
RedisLockRegistry.this.redisTemplate.delete(this.lockKey);
protected boolean removeLockKeyInnerDelete() {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.clientId));
}

}
Expand Down