When one of the Redis cluster nodes is crashed, its evictionTimer thread is not closed at the same time

[leewcc]

Expected behavior

I‘m running redis in cluster mode. One of my master nodes’ gone down, after renewSlot, the broken node's evictionTimer thread should be closed otherwise it will cost thread leak.

Actual behavior

I fork the code and add a log to proof this behavior. In the clients.jedis.Connection.connect()

public void connect() {
     if (!isConnected()) {
      try {
        .......
      } catch (IOException ex) {
// here is the code I added
        LOGGER.error("Connect error. Host: " + host + " Port: " + port + ", Exception = " + ex.getMessage());
        broken = true;
        throw new JedisConnectionException(ex);
      }
    }
  }

When one of my clsuter nodes is down, after recover by cluster renewSlot, the evictionTimer thread is not cancelled, below error log continue printing:

[2019-12-09 10:35:48.146] [ERROR] [commons-pool-EvictionTimer] [redis.clients.jedis.Connection] >>> [%PARSER_ERROR[eventname]] msg=Connect error. Host: 127.0.0.1 Port: 6379, Exception = connect timed out
[2019-12-09 10:35:53.145] [ERROR] [commons-pool-EvictionTimer] [redis.clients.jedis.Connection] >>> [%PARSER_ERROR[eventname]] msg=Connect error. Host: 127.0.0.1 Port: 6379, Exception = connect timed out
[2019-12-09 10:35:58.146] [ERROR] [commons-pool-EvictionTimer] [redis.clients.jedis.Connection] >>> [%PARSER_ERROR[eventname]] msg=Connect error. Host: 127.0.0.1 Port: 6379, Exception = connect timed out
[2019-12-09 10:36:03.147] [ERROR] [commons-pool-EvictionTimer] [redis.clients.jedis.Connection] >>> [%PARSER_ERROR[eventname]] msg=Connect error. Host: 127.0.0.1 Port: 6379, Exception = connect timed out

Steps to reproduce:

1. new a JedisCluster

GenericObjectPoolConfig config = new GenericObjectPoolConfig();
config.setMinIdle(1);
config.setMaxIdle(2);
config.setTestWhileIdle(true);
config.setTimeBetweenEvictionRunsMillis(5000L);
jedisCluster = new JedisCluster(new HostAndPort("127.0.0.1", 6379), config);

2. kill a cluster node
3. invoke get request to broken node to trigger renewSlot
4. view log

Redis / Jedis Configuration

Jedis version:2.9.0

Redis version: Redis 3.0.7

Java version:jdk 1.8.0_144

[wwr]

The renewSlotCache method appends the new cluster node rediscovered when topology changed to the nodes map cache, while it does not remove the offline nodes or clear the nodes map cache. See setupNodeIfNotExist for detail. This case, testWhileIdle was true and TimeBetweenEvictionRunsMillis was set 5000ms, the evictor thread will be executed every 5000ms to evict idle jedis and create jedis connection to nodes if needed to ensure minIdle.

[github-actions]

This issue is marked stale. It will be closed in 30 days if it is not updated.