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 RAILS_MIN_THREADS, RAILS_MAX_THREADS, set default worker, preload… #2143

Merged
merged 17 commits into from Apr 21, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions History.md
Expand Up @@ -10,6 +10,11 @@
* Changed #connected_port to #connected_ports (#2076)
* `--control` has been removed. Use `--control-url` (#1487)
* `worker_directory` has been removed. Use `directory`
* min_threads now set by environment variables RAILS_MIN_THREADS and MIN_THREADS
jalevin marked this conversation as resolved.
Show resolved Hide resolved
jalevin marked this conversation as resolved.
Show resolved Hide resolved
* max_threads now set by environment variables RAILS_MAX_THREADS and MAX_THREADS
* max_threads default to 5 in MRI or 16 for all other interpretters
* preload by default if workers > 1 and interpretter supports workers


* Bugfixes
* Windows update extconf.rb for use with ssp and varied Ruby/MSYS2 combinations (#2069)
Expand Down
13 changes: 10 additions & 3 deletions lib/puma/configuration.rb
Expand Up @@ -137,6 +137,8 @@ def initialize(user_options={}, default_options = {}, &block)
@file_dsl = DSL.new(@options.file_options, self)
@default_dsl = DSL.new(@options.default_options, self)

default_options[:preload_app] = (default_options[:workers] > 1 && Puma::Plugin.new.workers_supported?)
jalevin marked this conversation as resolved.
Show resolved Hide resolved

if block
configure(&block)
end
Expand Down Expand Up @@ -167,14 +169,19 @@ def flatten!
self
end

def default_max_threads
return 5 if Puma.mri?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a ternary here instead of return.

16
end

def puma_default_options
{
:min_threads => 0,
:max_threads => 16,
:min_threads => Integer(ENV['RAILS_MIN_THREADS'] || ENV['MIN_THREADS'] || 0),
:max_threads => Integer(ENV['RAILS_MAX_THREADS'] || ENV['MAX_THREADS'] || default_max_threads),
:log_requests => false,
:debug => false,
:binds => ["tcp://#{DefaultTCPHost}:#{DefaultTCPPort}"],
:workers => 0,
:workers => Integer(ENV['WEB_CONCURRENCY'] || 0),
:daemon => false,
jalevin marked this conversation as resolved.
Show resolved Hide resolved
:mode => :http,
:worker_timeout => DefaultWorkerTimeout,
Expand Down
4 changes: 4 additions & 0 deletions lib/puma/detect.rb
Expand Up @@ -12,4 +12,8 @@ def self.jruby?
def self.windows?
IS_WINDOWS
end

def self.mri?
RUBY_ENGINE == 'ruby' || RUBY_ENGINE.nil?
end
end
52 changes: 52 additions & 0 deletions test/test_config.rb
Expand Up @@ -8,6 +8,58 @@
class TestConfigFile < TestConfigFileBase
parallelize_me!

def test_default_max_threads
max_threads = 16
max_threads = 5 if RUBY_ENGINE.nil? || RUBY_ENGINE == 'ruby'
assert_equal max_threads, Puma::Configuration.new.default_max_threads
end

def test_config_loads_correct_min_threads
conf = Puma::Configuration.new
assert_equal 0, conf.options.default_options[:min_threads]

ENV['MIN_THREADS'] = '7'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting a global and running tests in parallel threads, you're gonna have a Bad Time (as you've seen on your tests).

Maybe just removing parallelize_me will fix everything here. Put these tests into a separate test class which is not paralellized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like removing parallelize_me! works here. Any other ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that- extrapolated into it's own class and used the with_env helper. All seems fine.

conf = Puma::Configuration.new
assert_equal 7, conf.options.default_options[:min_threads]

ENV['RAILS_MIN_THREADS'] = '8'
conf = Puma::Configuration.new
assert_equal 8, conf.options.default_options[:min_threads]
end

def test_config_loads_correct_max_threads
conf = Puma::Configuration.new
assert_equal conf.default_max_threads, conf.options.default_options[:max_threads]

ENV['MAX_THREADS'] = '7'
conf = Puma::Configuration.new
assert_equal 7, conf.options.default_options[:max_threads]

ENV['RAILS_MAX_THREADS'] = '8'
conf = Puma::Configuration.new
assert_equal 8, conf.options.default_options[:max_threads]
end

def test_config_loads_correct_workers
conf = Puma::Configuration.new
assert_equal 0, conf.options.default_options[:workers]

ENV['WEB_CONCURRENCY'] = '8'
conf = Puma::Configuration.new
assert_equal 8, conf.options.default_options[:workers]
end

def test_config_preloads_app_if_using_workers
ENV['WEB_CONCURRENCY'] = '0'
conf = Puma::Configuration.new
assert_equal false, conf.options.default_options[:preload_app]

ENV['WEB_CONCURRENCY'] = '2'
preload = Puma::Plugin.new.workers_supported?
conf = Puma::Configuration.new
assert_equal preload, conf.options.default_options[:preload_app]
end

def test_app_from_rackup
conf = Puma::Configuration.new do |c|
c.rackup "test/rackup/hello-bind.ru"
Expand Down