mperham / connection_pool

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

ConnectionPool::TimedStack is now lazy #52

Closed drbrain closed 10 years ago

drbrain commented 10 years ago

Now TimedStack only creates connections when the internal queue is empty and the number of connections is less than the maximum set. By default no connections are created when the pool is created.

This required some changes to the TimedStack tests because I wrote poor tests the first time around. The new tests better exercise the intended behavior of TimedStack.

This required some changes to the ConnectionPool tests to use up connections because now an unused pool has no connections.

mperham commented 10 years ago

Changing the semantics of the connection pool from eager to lazy is a very big semantic change and something worthy of a 2.0 version bump. I suspect many people depend on the behavior implicitly. I've not been a fan of lazy stuff when it comes to multi-threaded code as there's lots of room for race conditions. Perhaps I'm being too much of a crotchety old man.

drbrain commented 10 years ago

Future steps in this work are (in no particular order):

The ultimate goal is to use connection_pool in net-http-persistent. connection_pool would handle handing out connections for whatever host the user needed to talk to while reaping connections that weren't recently used.

This could work with both a small number of connections (for connecting to an API) or a large number of connections (for web spider work) since connection_pool has accounting built-in.

When I'm done I think my changes will be 2.0-worthy. How would you best like to review my changes? The changes I've mentioned above will need to build atop each other which doesn't lend itself to pull requests particularly well.

mperham commented 10 years ago

I still think PRs are the best approach, you can always target a staging branch, rather than master, so your changes can all be merged at once when the entire development process is complete.

mperham commented 10 years ago

Would you bump the version to 2.0 and update the changelog?

coelhone commented 10 years ago

lazy loading? sounds familiar...