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
Pass queued requests to thread pool on server shutdown #2122
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -185,8 +185,6 @@ def run(background=true) | |
|
||
@status = :run | ||
|
||
queue_requests = @queue_requests | ||
|
||
@thread_pool = ThreadPool.new(@min_threads, | ||
@max_threads, | ||
::Puma::IOBuffer) do |client, buffer| | ||
|
@@ -197,7 +195,7 @@ def run(background=true) | |
process_now = false | ||
|
||
begin | ||
if queue_requests | ||
if @queue_requests | ||
process_now = client.eagerly_finish | ||
else | ||
client.finish(@first_data_timeout) | ||
|
@@ -230,7 +228,7 @@ def run(background=true) | |
|
||
@thread_pool.clean_thread_locals = @options[:clean_thread_locals] | ||
|
||
if queue_requests | ||
if @queue_requests | ||
@reactor = Reactor.new self, @thread_pool | ||
@reactor.run_in_thread | ||
end | ||
|
@@ -314,11 +312,12 @@ def handle_servers | |
|
||
@events.fire :state, @status | ||
|
||
graceful_shutdown if @status == :stop || @status == :restart | ||
if queue_requests | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume this needs to be @queue_requests given your change. But I'd also rather you not change this to an ivar read and leave it as a local var. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were actually two separate The local var in That said, since this local var in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This explanation works for me. |
||
@queue_requests = false | ||
@reactor.clear! | ||
@reactor.shutdown | ||
end | ||
graceful_shutdown if @status == :stop || @status == :restart | ||
rescue Exception => e | ||
STDERR.puts "Exception handling servers: #{e.message} (#{e.class})" | ||
STDERR.puts e.backtrace | ||
|
@@ -388,6 +387,7 @@ def process_client(client, buffer) | |
end | ||
|
||
unless client.reset(check_for_more_data) | ||
return unless @queue_requests | ||
close_socket = false | ||
client.set_timeout @persistent_timeout | ||
@reactor.add client | ||
|
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.
This removes a change from cc4ad17, since we need to change behavior based on the updated value of the
@queue_requests
variable after the reactor is shutdown. I assume this was a performance optimization, but I don't know if it actually makes any measurable difference.