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

Add option 'worker_shutdown' #171

Merged
merged 2 commits into from May 15, 2020

Conversation

sirihansen
Copy link
Contributor

The shutdown option in the child specs for each worker is hardcoded to 5000 ms. If something is slow during shutdown, this may add up to quite a lot depending on the number of workers. In my case I want to allow the workers to empty their inboxes on controlled termination so I don't want to use the default brutal_kill for the supervisor, but I also don't want to allow too much time if something hangs or is slow during this cleanup.

This PR includes:

  • The new option worker_shutdown which can have the value brutal_kill | timeout()
  • Add short description of option pool_sup_shutdown to README.md

Allow changing the default of 5000ms for shutdown time of each single
worker.
@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #171 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #171   +/-   ##
=======================================
  Coverage   96.35%   96.36%           
=======================================
  Files          10       10           
  Lines         439      440    +1     
=======================================
+ Hits          423      424    +1     
  Misses         16       16           
Impacted Files Coverage Δ
src/wpool_process_sup.erl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90af3ad...cb94d4e. Read the comment docs.

README.md Outdated Show resolved Hide resolved
@elbrujohalcon
Copy link
Member

@sirihansen I'm inclined to merge this PR, but I'll wait in case you want to fix the description in the readme as you proposed.

@elbrujohalcon
Copy link
Member

No need for squashing. Thanks, @sirihansen :)
I'll merge and generate a new release later today.

@elbrujohalcon elbrujohalcon merged commit c9544af into inaka:master May 15, 2020
@sirihansen sirihansen deleted the worker-shutdown-option branch May 15, 2020 09:12
@elbrujohalcon
Copy link
Member

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.

None yet

2 participants