taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 131 forks source link

Commons-pool can slowly lose connections over time #127

Closed cespare closed 9 years ago

cespare commented 9 years ago

Hi @ptaoussanis,

This is not, strictly speaking, a bug in Carmine, but we thought you would like to know. We have a server with many Redis connections and we found that over time, fewer and fewer were available to Carmine until the applications deadlocks waiting on connections. (This may be the same as #112; our stack traces certainly look the same.)

We did a lot of digging, and found that commons-pool2's GenericKeyedObjectPool can lose connections due to a bug in the eviction code. Carmine enables the evictor by default (the "jedis settings"). If we disable it, then we sometimes are handed connections that have been timed out by our redis server.

We've filed a bug against commons-pool2:

https://issues.apache.org/jira/browse/POOL-287

After spending many hours debugging and sifting through commons-pool2 code, we were frankly astounded by its complexity and subtlety, and we do not wish to continue depending on it in our production code. On our end, we've replaced commons-pool2 by a much simpler object pool of our own design. (Thanks for making the connection pool pluggable.)

ptaoussanis commented 9 years ago

Hi Caleb, thanks for bringing this to my attention (and for the report to Apache!). Will keep an eye on the Apache issue as it develops.

Cheers!

vedang commented 9 years ago

Hi @cespare,

We're also possibly hitting this bug. Can you share your clojure code to reproduce this issue (if it's not too much trouble)?

cespare commented 9 years ago

@vedang My reproducer only uses commons-pool, not Carmine. Is that what you're interested in?

vedang commented 9 years ago

@cespare Yes, I'd like to see a local repro of the issue. Your code will be handy to me :)

dparis commented 9 years ago

Just FYI, looks like this was fixed in 2.4 a couple days ago.

jvah commented 9 years ago

I think that means will be fixed in 2.4, latest version out is still 2.3. But this bug hunting is great effort, big :+1: to @cespare

dparis commented 9 years ago

@jvah Yup, I don't think 2.4 has been released yet, but the fix is done so it's just a matter of time now. I mentioned it here only as a heads-up to the people following this thread.

And yes, thanks to everyone for their work on this library. :clap:

ptaoussanis commented 9 years ago

@dparis Thanks for the update.

Am ready to cut a new Carmine release as soon as commons-pool v2.4 is out.

ptaoussanis commented 9 years ago

Quick update just to mention that commons-pool v2.4 still isn't out: http://mvnrepository.com/artifact/org.apache.commons/commons-pool2

Will continue to keep an eye out for its release.

mrmemes-eth commented 9 years ago

Just a heads-up: it looks like 2.4.1 is out.

ptaoussanis commented 9 years ago

@voxdolo Thanks for the ping, will try cut a release later today!

ptaoussanis commented 9 years ago

Just pushed [com.taoensso/carmine "2.11.0"] to Clojars, closing.

@cespare Thanks again for taking care of this, cheers! :+1:

mrmemes-eth commented 9 years ago

Thanks @ptaoussanis and @cespare :)