CodisLabs / jodis

A java client for codis based on Jedis and Curator
MIT License
217 stars 97 forks source link

Fix "use after closing" error when calling pool.getResource() #60

Closed william-cheung closed 5 years ago

william-cheung commented 5 years ago

There exists race condition between pool.close() and pool.getResource(), which may cause the following exception: redis.clients.jedis.exceptions.JedisConnectionException: Could not get a resource from the pool at redis.clients.util.Pool.getResource(Pool.java:53) at redis.clients.jedis.JedisPool.getResource(JedisPool.java:226) at io.codis.jodis.RoundRobinJedisPool.getResource(RoundRobinJedisPool.java:220) ...

I proposed two solutions to this problem:

  1. defer "close" of a pool; 2. close the pool when PooledObject.finalize() is called

The first solution may cause errors under conditions that rarely happen, but it is more deterministic than the second on resource collection. I prefer the first one.

william-cheung commented 5 years ago

This fix will enable rolling-restart of codis proxies without affecting codis clients. The rolling-restart process should be like:

for proxy in proxies:
      mark_offline(proxy)  # set the status of 'proxy' to 'offline'
      sleep(10)  # wait 10 seconds for all ongoing requests to 'proxy' to be completed
      restart(proxy)  # kill 'proxy' and restart it