Skip to content

Drop switch from Redis unlink to delete #10460

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 3, 2025
Merged

Drop switch from Redis unlink to delete #10460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb6dead
Carefully switch from Redis unlink to delete
Oct 3, 2025
5625cdd
Drop fallback from Redis unlink to delete
Oct 3, 2025
013d43a
Drop fallback from Redis unlink to delete
Oct 3, 2025
ca952c6
Checkstyle
Oct 3, 2025
e317b2b
Checkstyle
Oct 3, 2025
460616c
Author
Oct 3, 2025
415d7a8
Same cleanup in RedisMessageStore
Oct 3, 2025
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
41 changes: 3 additions & 38 deletions ...on-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* @author Oleg Zhurakousky
* @author Gary Russell
* @author Artem Bilan
* @author Michal Domagala
*
* @since 2.1
*/
Expand All @@ -48,8 +49,6 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B

private boolean valueSerializerSet;

private volatile boolean unlinkAvailable = true;

/**
* Construct {@link RedisMessageStore} based on the provided
* {@link RedisConnectionFactory} and default empty prefix.
Expand Down Expand Up @@ -133,48 +132,14 @@ protected Object doRemove(Object id) {
Assert.notNull(id, ID_MUST_NOT_BE_NULL);
Object removedObject = this.doRetrieve(id);
if (removedObject != null) {
if (this.unlinkAvailable) {
try {
this.redisTemplate.unlink(id);
}
catch (Exception ex) {
unlinkUnavailable(ex);
this.redisTemplate.delete(id);
}
}
else {
this.redisTemplate.delete(id);
}
this.redisTemplate.unlink(id);
}
return removedObject;
}

private void unlinkUnavailable(Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
}
else {
logger.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command: " + ex.getMessage());
}
this.unlinkAvailable = false;
}

@Override
protected void doRemoveAll(Collection<Object> ids) {
if (this.unlinkAvailable) {
try {
this.redisTemplate.unlink(ids);
}
catch (Exception ex) {
unlinkUnavailable(ex);
this.redisTemplate.delete(ids);
}
}
else {
this.redisTemplate.delete(ids);
}
this.redisTemplate.unlink(ids);
}

@Override
Expand Down
90 changes: 8 additions & 82 deletions ...ion-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
* @author Roman Zabaluev
* @author Alex Peelman
* @author Youbin Wu
* @author Michal Domagala
*
* @since 4.0
*
Expand Down Expand Up @@ -158,8 +159,6 @@ protected boolean removeEldestEntry(Entry<String, RedisLock> eldest) {
*/
private boolean executorExplicitlySet;

private volatile boolean unlinkAvailable = true;

private volatile boolean isRunningRedisMessageListenerContainer = false;

/**
Expand Down Expand Up @@ -436,11 +435,6 @@ protected abstract boolean tryRedisLockInner(long time, long expireAfter)
*/
protected abstract boolean removeLockKeyInnerUnlink();

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

@Override
public final void lock() {
this.lock(RedisLockRegistry.this.expireAfter);
Expand Down Expand Up @@ -583,41 +577,15 @@ public final void unlock() {
}

private void removeLockKey() {
if (RedisLockRegistry.this.unlinkAvailable) {
Boolean unlinkResult = null;
try {
// Attempt to UNLINK the lock key; an exception indicates lack of UNLINK support
unlinkResult = removeLockKeyInnerUnlink();
}
catch (Exception ex) {
RedisLockRegistry.this.unlinkAvailable = false;
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command", ex);
}
else {
LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " +
"falling back to the regular DELETE command: " + ex.getMessage());
}
}

if (Boolean.TRUE.equals(unlinkResult)) {
// Lock key successfully unlinked
stopRenew();
return;
}
else if (Boolean.FALSE.equals(unlinkResult)) {
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
boolean unlinkResult = removeLockKeyInnerUnlink();
if (unlinkResult) {
// Lock key successfully removed
stopRenew();
}
if (!removeLockKeyInnerDelete()) {
else {
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
else {
stopRenew();
}
}

protected final boolean renew(long expireAfter) {
Expand Down Expand Up @@ -705,21 +673,9 @@ private final class RedisPubSubLock extends RedisLock {
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);

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

private RedisPubSubLock(String path) {
super(path);
}
Expand All @@ -732,17 +688,8 @@ protected boolean tryRedisLockInner(long time, long expireAfter)

@Override
protected boolean removeLockKeyInnerUnlink() {
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
}

@Override
protected boolean removeLockKeyInnerDelete() {
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
}

private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
redisScript, Collections.singletonList(this.lockKey),
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey));
}

Expand Down Expand Up @@ -854,21 +801,9 @@ private final class RedisSpinLock extends RedisLock {
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 @@ -894,17 +829,8 @@ protected boolean tryRedisLockInner(long time, long expireAfter) throws Interrup

@Override
protected boolean removeLockKeyInnerUnlink() {
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
}

@Override
protected boolean removeLockKeyInnerDelete() {
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
}

private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
redisScript, Collections.singletonList(this.lockKey),
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
RedisLockRegistry.this.clientId));
}

Expand Down