taoensso / carmine

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

Whats the best way to close the connection pool ? #224

Closed spariev closed 1 year ago

spariev commented 5 years ago

I am deploying an app which uses the carmine on Tomcat and on app server shutdown I see the following warning -

The web application [ROOT##201906061327] appears to have started a thread named [commons-pool-EvictionTimer] but has failed to stop it. This is very likely to create a memory leak.

I looked into carmine code and can't figure out how to close the underlying connection pool, as I cant get a handle on it via the existing API. Can you please advice me how to deal with the problem?

CmdrDats commented 4 years ago

I have a similar issue - My use case is making sure all threads close themselves cleanly, both for use in CLI tool (clean natural exit without System/exit) and for managing context services (like mysql, kafka, redis, etc) so that I can cleanly restart without resources floating about and without actually restarting the JVM process.

I have tried to (rconn/close-conn (second (rconn/pooled-conn state))) (where state is the {:spec ...}) but it doesn't seem to close it - if I add :pool :none into the config in the first place, that does work correctly, but obviously because it doesn't even create a connection pool in the first place.

What I have found that works is: (.close ^Closeable (second (rconn/pooled-conn state)))) - but it feels a bit tenuous, since I'm not entirely sure there's nothing else floating about? Ideally, I could get the connection pool directly when I first setup the Redis connection and use that instead of the spec map in wcar calls - and then directly close that when I'm done?

aphyr commented 4 years ago

I'd like to second this confusion: the fact that Carmine (permanently?) squirrels away connection pools via memoization makes it difficult to tell when and how you can close a connection pool safely, if ever. The memoization thing really worries me, because it feels like you have a choice between uncontrollable sharing of mutable state vs using different :ids and cultivating an ever-growing graveyard of memoized connection pools.

ptaoussanis commented 1 year ago

Hi all, apologies for taking forever to reply to this. Connection pools can be closed with the java.io.Closeable method:

(defonce my-pool (carmine.connections/conn-pool :mem/fresh {})) ; Manually create a pool, bypass cache
(wcar {:pool my-pool} (ping)) ; Use the pool
(.close my-pool) ; Close the pool
(wcar {:pool my-pool} (ping)) ; Will throw, pool is closed

Pool memoization won't be relevant if you're providing your own pool like this.

Apologies for any confusion, the current connections API is poorly documented and can be awkward. This is an area that's seeing a lot of attention for the next major Carmine version.

Cheers!

CmdrDats commented 1 year ago

Thanks for the update and all the hard work!

ptaoussanis commented 1 year ago

@CmdrDats You're very welcome!

Quick update: since v4 is still a while away, I've just pushed an intermediate v3.2 release which includes a clarified wcar docstring that should hopefully help with the confusion here.

A pool constructor is now also aliased directly in the core Carmine ns, making it more obvious for folks that want to provide a manually-constructed pool.

Cheers!

theferrit32 commented 1 year ago

@ptaoussanis this is great, thanks for the clarification and update and your work on carmine in general

spariev commented 1 year ago

@ptaoussanis great to hear! Updated docstring is indeed helpful, thanks for your work on this issue!