Drop switch from Redis unlink to delete

Redis 3, where no `UNLINK` yet, is not maintained since 2018

Fix `RedisLockRegistry` and `RedisMessageStore` to use only `UNLIK` for entries removal.
The `DEL` fallback does not make sense with modern Redis versions.
More over, such an algorithm was premature and felt back to `DEL` permanently for any errors.

Signed-off-by: Michal Domagala <[email protected]>

Author: michaldo

spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java

@@ -37,6 +37,7 @@
  * @author Oleg Zhurakousky
  * @author Gary Russell
  * @author Artem Bilan
+ * @author Michal Domagala
  *
  * @since 2.1
  */
@@ -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.
@@ -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

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

@@ -96,6 +96,7 @@
  * @author Roman Zabaluev
  * @author Alex Peelman
  * @author Youbin Wu
+ * @author Michal Domagala
  *
  * @since 4.0
  *
@@ -158,8 +159,6 @@ protected boolean removeEldestEntry(Entry<String, RedisLock> eldest) {
 	 */
 	private boolean executorExplicitlySet;
 
-	private volatile boolean unlinkAvailable = true;
-
 	private volatile boolean isRunningRedisMessageListenerContainer = false;
 
 	/**
@@ -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);
@@ -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) {
@@ -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);
 		}
@@ -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));
 		}
 
@@ -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);
 		}
@@ -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));
 		}