search

mperham / connection_pool

Generic connection pooling for Ruby
MIT License
1.63k stars 143 forks source link

Add reap functionality #187

Closed ttstarck closed 3 months ago

ttstarck commented 3 months ago

There have been several discussions/attempts at adding a idle connection reaping functionality to this gem: https://github.com/mperham/connection_pool/issues/125 https://github.com/mperham/connection_pool/pull/127 https://github.com/mperham/connection_pool/pull/126

I and many others would find value in adding this functionality however I am in agreement with the previous discussions that the key to adding this reap functionality is to add it in a way that keeps the simplicity of this gem.

In order to accomplish this, I've added two new public methods to ConnectionPool (as well as matching methods for TimedStack):

  1. #idle - Returns how many connections that are created and available in the pool. This is different than available because available includes connections that haven't been created yet.
  2. #reap(idle_seconds = 0, &block) - Removes connections that have been idle for longer than idle_seconds and yields each connection to the block passed to the method in order to close/clean up the connection. By default, idle_seconds is 0, which means #reap will remove all connections that are on the stack.

In order to implement the #reap method in connection pool, I've changed how the TimedStack queue stores connections in the queue to also include the current time when pushed back onto the stack.

mperham commented 3 months ago

I’m wondering if the default idle_seconds value should be something more like 60. 0 is very aggressive.

ttstarck commented 3 months ago

I’m wondering if the default idle_seconds value should be something more like 60. 0 is very aggressive.

Good call, 60 seconds is a more sane value such that this method can be called and not cause thrash of opening/closing connections. Fixed https://github.com/mperham/connection_pool/pull/187/commits/c4f5a6c9ff2f016f6ae97f88c0005ab4e90b78fa

mperham commented 3 months ago

Did you document how the user can implement reaping?

ttstarck commented 3 months ago

Did you document how the user can implement reaping?

Updated the README with information on using #reap and #idle methods: https://github.com/mperham/connection_pool/pull/187/commits/2148a2881eb8b7afbda969d64ad3364841eac792

I could also add a section on implementing a reaper thread 🤔... Is that what you are referring to or is what I added above enough.

mperham commented 3 months ago

Yeah, I meant with a Thread. Your code example isn't quite realistic yet, I'd change it to put the reap call in a Thread.

ttstarck commented 3 months ago

Sorry for the delay:

I've added a new section about creating your own reaper thread to the README:

https://github.com/mperham/connection_pool/pull/187/commits/28e9760794e0cfb640e21a3f90281489ff375ae8

mperham commented 3 months ago

This is great, nice work. I’ll play with it later and push out a new release in a few weeks.

shayonj commented 2 months ago

Excited for this, thank you!