Introduction of unlink with lock broke integrations with Twemproxy

[nbroeking]

Bug report / Enhancement

We use spring-integration-redis to use twemproxy or T-Redis. (T-Redis is a sharding mechanism for Redis). The library worked great until the introduction of the following diff.

88c7646#diff-befc3689f434cc94881bf3d1818f9191

Once this diff was released it causes an exception to be thrown when you issue an lock.unlock() call. This happens because twemproxy does not support the call info server.

The last working version was 5.0.1. All of the more recent versions are broken.

Is this something you support / are willing to fix?

[garyrussell]

@nbroeking Can you provide a stack trace so we can figure out the best way to resolve this?

[artembilan]

This goes from here:

/**
	 * Perform an {@code INFO} command on the provided {@link RedisOperations} to check
	 * the Redis server version to be sure that {@code UNLINK} is available or not.
	 * @param redisOperations the {@link RedisOperations} to perform {@code INFO} command.
	 * @return true or false if {@code UNLINK} Redis command is available or not.
	 * @throws IllegalStateException when {@code INFO} returns null from the Redis.
	 */
	public static boolean isUnlinkAvailable(RedisOperations<?, ?> redisOperations) {
		return unlinkAvailable.computeIfAbsent(redisOperations, key -> {
			Properties info = redisOperations.execute(
					(RedisCallback<Properties>) connection -> connection.serverCommands().info(SECTION));
			if (info != null) {
				String version = info.getProperty(VERSION_PROPERTY);
				if (StringUtils.hasText(version)) {
					int majorVersion = Integer.parseInt(version.split("\\.")[0]);
					return majorVersion >= 4;
				}
				else {
					return false;
				}
			}
			else {
				throw new IllegalStateException("The INFO command cannot be used in pipeline/transaction.");
			}
		});
	}

See that connection.serverCommands().info(SECTION)).
That's weird that there are some clients, but yeah, we definitely need to fix.
So, stack trace is really good to have for better tracking.

Thanks

[garyrussell]

Perhaps we could fall back to do a trial unlink if we can't get the version?

[nbroeking]

Hey @garyrussell and @artembilan for some reason my notification settings were broken. I just saw your comments. Thank you for addressing. Is there anything else you need from me?

[artembilan]

Thank you, @nbroeking, for coming back to us! Well, that fix has been merged. You can test your solution with the latest snapshot

[FranPregernik]

Hi!

#nitpick I would like to propose to write the log warning with just the exception message and not the whole stack trace.
Maybe add an logger.isDebugEnabled and then ouput the whole exception stack.

We have an older Redis and every lock now outputs an exception stack trace dump.

Best regards!

[artembilan]

@FranPregernik ,

please, raise a new GH issue.
I think the problem is not about logging level, but how we deal with the unlinkAvailable state.
It must be on the RedisLockRegistry level, not on each RedisLock instance.

Although your request about debug logging mode makes sense, too.

Anyway: a new issue, please. This one is fully not related anymore.