JedisConnection.scan releases the connection instance to the pool [DATAREDIS-604]

[spring-projects-issues]

Stefan Müller opened DATAREDIS-604 and commented

When using JedisConnection.scan to iterate over the redis database keys and closing the cursor afterwards the JedisConnection is closed and released to the pool. If you try to release the connection afterwards an exception is thrown.

long count = 0;
RedisConnection connection = null;
try {
	connection = RedisConnectionUtils.getConnection(redisConnectionFactory);
	try (Cursor<byte[]> entries = connection
			.scan(ScanOptions.scanOptions().match(REFRESH + "*").count(1000).build())) {
		while (entries.hasNext()) {
			count++;
			entries.next();
		}
	} catch (IOException e) {
		log.warn("error closing redis cursor", e);
	}
} finally {
	if ((connection != null) && (!connection.isClosed())) {
		RedisConnectionUtils.releaseConnection(connection, redisConnectionFactory);
	}
}

I tracked down the problem to the following pull request:
#218

I understand that the connections need to be released when scan is used from RedisTemplate.getXXXOperations - but when using the connection directly releasing the connection should be done by the caller.

As a workaround we must catch the exception which is thrown

redis.clients.jedis.exceptions.JedisException: Could not return the resource to the pool
Caused by: java.lang.IllegalStateException: Object has already been returned to this pool or is invalid

to make our code work with spring-data-redis 1.7 and 1.8 because there is no way to detect if a connection is already returned to the pool / factory.

I would like to know if this is a bug or intended behavior. If it is intended behavior it should be documented, that closing the scan-cursor closes the underlying connection and the caller needs to make a new connection to continue

---

Affects: 1.8 GA (Ingalls)

[spring-projects-issues]

Mark Paluch commented

Don't use try-with-resources if you require cross-version compatibility and release the connection yourself. Our change is intentional due to Cursor's deferred execution and to not break existing behavior. A cursor must remain functional if it's passed outside a method.

Your suggestion makes sense to leave resource cleanup up to the user when using RedisConnection directly and add the cleanup aspect to Cursor if a scan command is used through the Template API

[spring-projects-issues]

Yanming Zhou commented

Mark Paluch Should I use try-with-resources with latest spring-data-redis and lettuce driver?

Set<String> hosts = stringRedisTemplate.<Set<String>>execute((RedisConnection conn) -> {
	Set<String> set = new HashSet<>();
	try (Cursor<byte[]> cursor = conn
			.scan(new ScanOptions.ScanOptionsBuilder().match("host:*").count(100).build())) {
		while (cursor.hasNext())
			set.add((String) stringRedisTemplate.getKeySerializer().deserialize(cursor.next()));
	} catch (IOException e) {
		throw new RuntimeException(e);
	}
	return set;
});

[spring-projects-issues]

Mark Paluch commented

You don't have to if you call SCAN within execute(…) as the execute(…) method takes care of resource cleanup