-
-
Notifications
You must be signed in to change notification settings - Fork 280
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
RIP Celluloid, Hello concurrent-ruby/Thread pool #291
Merged
Merged
Changes from 42 commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
f1df7ab
WIP
1f3d3c4
WIP simple thread pool
b8f0830
Clean up celluloid references
ca8504f
Fix hard shutdown.
cb2a0e7
Clean TTIN
be67d4b
Implement AutoExtendVisibility using Concurrent::TimerTask
85c612d
Remove no longer needed methods
ad0a57e
Only delay hard shutdown if there's any busy worker
a33838c
Remove sleep used for debugging
ca15687
Clean up
da429bc
Fix specs
ff73931
Disable brakeman
e8f784f
Merge branch 'master' into thread-pool
dd9bfb4
Merge branch 'master' into thread-pool
606f0ca
Merge branch 'master' into thread-pool
ffd3517
Merge branch 'master' into thread-pool
79705a1
Remove safe navigation operator it isn't supported prior Ruby 2.3 :sad:
7315786
Delay is no longer needed for soft shutdown
dfee6d4
Wrap dispatch_later with a mutex to avoid concurrency comming from the
f20b676
Naming refactor
3bad1e0
watchdog is no longer needed
9b9d4b3
Remove Mutex in favor of `make_true`
4bd68ea
Turn dispatch/dispatch_later into a heartbeat
ef295c8
Fix specs
474838e
Use `Concurrent::TimerTask` as a heartbeat
8cd2817
Clean up
90a1363
Clean auto extend visibility
7803300
Better hard_shutdown_in handling with wait_for_termination
3962069
Naming refactor
eb2edff
Decrease the heartbeat interval to 0.25
8aa9124
Test PutsReq
766fe99
Bump delay to 1 sec
9563b08
Change delay to 0.10
91c3c2f
Add screenshots
f61c566
Dispatch when done
5ebfed5
non-stop dispatch
ac52dab
Dispatch when done
1fc4146
Test wait_time_seconds:10
1eb7092
execution_interval: 0.05
59bc99b
execution_interval: 0.15
540b45a
A manager every 10 concurrency
7331989
Test performance 1 processor 1 fetcher
93bb041
Stop logging out when 0 messages found.
f75a943
Clean up PutsReq tests
fed9d81
:lipstick:
c36d58f
Merge branch 'master' into thread-pool
7f18e02
Fix spec
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
.bundle | ||
.config | ||
.yardoc | ||
Gemfile.lock | ||
# Gemfile.lock | ||
InstalledFiles | ||
_yardoc | ||
coverage | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
PATH | ||
remote: . | ||
specs: | ||
shoryuken (2.1.1) | ||
aws-sdk-core (~> 2) | ||
concurrent-ruby | ||
|
||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
aws-sdk-core (2.6.32) | ||
aws-sigv4 (~> 1.0) | ||
jmespath (~> 1.0) | ||
aws-sigv4 (1.0.0) | ||
byebug (9.0.5) | ||
codeclimate-test-reporter (1.0.3) | ||
simplecov | ||
coderay (1.1.1) | ||
concurrent-ruby (1.0.2) | ||
diff-lcs (1.2.5) | ||
docile (1.1.5) | ||
dotenv (2.1.1) | ||
jmespath (1.3.1) | ||
json (2.0.2) | ||
method_source (0.8.2) | ||
mini_portile2 (2.1.0) | ||
multi_xml (0.5.5) | ||
nokogiri (1.6.8) | ||
mini_portile2 (~> 2.1.0) | ||
pkg-config (~> 1.1.7) | ||
pkg-config (1.1.7) | ||
pry (0.10.4) | ||
coderay (~> 1.1.0) | ||
method_source (~> 0.8.1) | ||
slop (~> 3.4) | ||
pry-byebug (3.4.0) | ||
byebug (~> 9.0) | ||
pry (~> 0.10) | ||
rake (11.2.2) | ||
rspec (3.5.0) | ||
rspec-core (~> 3.5.0) | ||
rspec-expectations (~> 3.5.0) | ||
rspec-mocks (~> 3.5.0) | ||
rspec-core (3.5.2) | ||
rspec-support (~> 3.5.0) | ||
rspec-expectations (3.5.0) | ||
diff-lcs (>= 1.2.0, < 2.0) | ||
rspec-support (~> 3.5.0) | ||
rspec-mocks (3.5.0) | ||
diff-lcs (>= 1.2.0, < 2.0) | ||
rspec-support (~> 3.5.0) | ||
rspec-support (3.5.0) | ||
simplecov (0.12.0) | ||
docile (~> 1.1.0) | ||
json (>= 1.8, < 3) | ||
simplecov-html (~> 0.10.0) | ||
simplecov-html (0.10.0) | ||
slop (3.6.0) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
DEPENDENCIES | ||
bundler (~> 1.6) | ||
codeclimate-test-reporter | ||
dotenv | ||
multi_xml | ||
nokogiri | ||
pry-byebug | ||
rake | ||
rspec | ||
shoryuken! | ||
simplecov | ||
|
||
BUNDLED WITH | ||
1.13.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
worker: bundle exec bin/shoryuken -r ./putsreq_worker.rb -q default | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,29 @@ | ||
module Shoryuken | ||
class Launcher | ||
include Celluloid | ||
include Util | ||
|
||
trap_exit :actor_died | ||
|
||
attr_accessor :manager | ||
|
||
def initialize | ||
@condvar = Celluloid::Condition.new | ||
@manager = Shoryuken::Manager.new_link(@condvar) | ||
count = Shoryuken.options.fetch(:concurrency, 25) | ||
|
||
@done = false | ||
raise(ArgumentError, "Concurrency value #{count} is invalid, it needs to be a positive number") unless count > 0 | ||
|
||
manager.fetcher = Shoryuken::Fetcher.new | ||
manager.polling_strategy = Shoryuken.options[:polling_strategy].new(Shoryuken.queues) | ||
@managers = Array.new(count) do | ||
Shoryuken::Manager.new(1, | ||
Shoryuken::Fetcher.new, | ||
Shoryuken.options[:polling_strategy].new(Shoryuken.queues)) | ||
end | ||
end | ||
|
||
def stop(options = {}) | ||
watchdog('Launcher#stop') do | ||
@done = true | ||
|
||
manager.async.stop(shutdown: !!options[:shutdown], timeout: Shoryuken.options[:timeout]) | ||
@condvar.wait | ||
manager.terminate | ||
end | ||
@managers.map do |manager| | ||
Thread.new { manager.stop(shutdown: !!options[:shutdown], timeout: Shoryuken.options[:timeout]) } | ||
end.each(&:join) | ||
end | ||
|
||
def run | ||
watchdog('Launcher#run') do | ||
manager.async.start | ||
end | ||
end | ||
|
||
def actor_died(actor, reason) | ||
return if @done | ||
logger.warn { "Shoryuken died due to the following error, cannot recover, process exiting: #{reason}" } | ||
exit 1 | ||
@managers.map do |manager| | ||
Thread.new { manager.start } | ||
end.each(&:join) | ||
end | ||
end | ||
end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside, have you considered not emitting this when the sqs_msgs are empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tjsingleton I haven't, but it seems to be a good idea. The only one issue is because the process will may look like isn't running/moving without that message, in case all queues are empty. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this for awhile and as long as the process lifecycle has good log messages, I don't think it's typically expected to "heartbeat" via the log. For instance, do you trust that puma is up and processing requests despite that it's doesn't log?