devinus / poolboy

A hunky Erlang worker pool factory
http://github.com/devinus/poolboy
ISC License
1.56k stars 348 forks source link

Improved logic for checking overflow workers #89

Open comtihon opened 8 years ago

comtihon commented 8 years ago

As discussed in https://github.com/devinus/poolboy/pull/83#issuecomment-224312388 I made improvements in zugolosian`s changes. In https://github.com/zugolosian/poolboy/pull/1 all commits are discussed. Summing up:

devinus commented 8 years ago

@fishcakez I could really use another set of eyes on this here.

fishcakez commented 8 years ago

When reaping in batches there is no need to read/store the monotonic time in the workers queue, only the time of the poll when it would be reaped or the time of the last reap. This need not be a clock time but could be a counter that increments once per reap, in a similar way to a timer wheel works. Using a counter (instead of monotonic time) may introduce some lag in the TTL but I don't think that will be an issue here because strict timing is not required. This would remove the OTP-18 requirement and should be faster. With OTP-18 strict timing would be possible using absolute timers.

This branch needs to access both ends of the worker queue so you may want to consider a different data structure. The fifo strategy already does a copy of the queue when inserting a worker.

fishcakez commented 8 years ago

I am not sure how I feel about adding features to poolboy because I think its main feature is that it is simple. Part of me feels that poolboy would benefit from removing the overflow feature because churn occurs surprisingly often and I advise all Ecto/Postgrex users not to set max_overflow. The churn means that workers need to created/destroy resources frequently which is more costly than the gains from dynamic resizing. If there isn't a resource and poolboy is just being used to limit concurrency then satefyvalve, jobs or sregulator would be more appropriate tools.