Add reaper for unused connections #127
Conversation
| @@ -32,7 +33,7 @@ | |||
| # - :timeout - amount of time to wait for a connection if none currently available, defaults to 5 seconds | |||
| # | |||
| class ConnectionPool | |||
| DEFAULTS = {size: 5, timeout: 5} | |||
| DEFAULTS = {size: 5, timeout: 5, reaping_frequency: nil, reap_after: 60} | |||
There was a problem hiding this comment.
the choice to make reaping_frequency the trigger to turn on reaping is to make it behave like ActiveRecord.
60s is a short reap_after in my opinion though.
| @@ -92,7 +105,14 @@ def checkin | |||
| nil | |||
| end | |||
|
|
|||
| # Removes a connection from the pool and makes the space available again | |||
| # connection may not currently be checked out of the queue. | |||
| def remove_connection(conn) | |||
There was a problem hiding this comment.
If this is called during a with block, or before checkin is called the connection will end up back in the queue.
This will not close the connection
| end | ||
|
|
||
| pool.with{} | ||
| sleep 2 |
There was a problem hiding this comment.
This isn't ideal, but I didn't want to introduce some mocking framework like timecop without feedback
|
I'll review the failing jruby issue |
|
I'm afraid you might have concurrency issues reaping under heavy load. I think reaping should be internal to TimedStack, which will allow you better control of concurrent access while reaping. You should need very minimal changes to ConnectionPool if you do it right. wdyt? |
Here is my first proposal for a connection reaper.
Test are added at the connection level, since test of the reaper class itself would require enough mocking to make it more fragile.
There is still race condition, where a connection is checked out during a reaping. The connection will be removed and then closed, odds are the consumer will encounter a runtime error at that point.
The only way to fix this would be to share the mutex, either by passing it into the reaper or bringing the reaping logic into the Connection Pool class.
I'm currently letting this soak in our test env at work to ensure it works as expected.