Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved logic for checking overflow workers #89

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

comtihon
Copy link

As discussed in #83 (comment) I made improvements in zugolosian`s changes.
In zugolosian#1 all commits are discussed.
Summing up:

  • overflow rip check is now hold periodically without mass timer creation
  • use monotonic time instead of os:timestamp
  • add improvement in case of lifo when checking old overflow workers

David Leach and others added 10 commits November 20, 2015 22:49
When workers are expensive to start and transactions are quick killing
all workers that are checked in when in overflow is very expensive.
This change allows delaying the termination of overflow workers when
there is peak load and alleviates worker churn.
When workers are expensive to start and transactions are quick killing
all workers that are checked in when in overflow is very expensive.
This change allows delaying the termination of overflow workers when
there is peak load and alleviates worker churn.
@devinus
Copy link
Owner

devinus commented Oct 2, 2016

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

@fishcakez
Copy link
Contributor

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
Copy link
Contributor

fishcakez commented Oct 2, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants