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 12 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
6 changes: 5 additions & 1 deletion History.md
Expand Up @@ -13,7 +13,11 @@
* Deprecations, Removals and Breaking API Changes
* `Puma.stats` now returns a Hash instead of a JSON string (#2086)
* `--control` has been removed. Use `--control-url` (#1487)
* `worker_directory` has been removed. Use `directory`.
* `worker_directory` has been removed. Use `directory`
* min_threads now set by environment variables PUMA_MIN_THREADS and MIN_THREADS
* max_threads now set by environment variables PUMA_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
* `tcp_mode` has been removed without replacement. (#2169)
* Daemonization has been removed without replacement. (#2170)
* Changed #connected_port to #connected_ports (#2076)
Expand Down
18 changes: 15 additions & 3 deletions lib/puma/configuration.rb
Expand Up @@ -137,6 +137,12 @@ def initialize(user_options={}, default_options = {}, &block)
@file_dsl = DSL.new(@options.file_options, self)
@default_dsl = DSL.new(@options.default_options, self)

workers_supported = !(Puma.jruby? || Puma.windows?)
Copy link
Member

Choose a reason for hiding this comment

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

Check ::Process.respond_to? :fork instead.

Copy link
Member

Choose a reason for hiding this comment

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

Also no need to assign to a variable, just put it on line 143


if !@options[:prune_bundler]
default_options[:preload_app] = (@options[:workers] > 1) && workers_supported
end

if block
configure(&block)
end
Expand Down Expand Up @@ -167,14 +173,20 @@ 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['PUMA_MIN_THREADS'] || ENV['MIN_THREADS'] || 0),
:max_threads => Integer(ENV['PUMA_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,
:worker_boot_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
9 changes: 6 additions & 3 deletions test/test_cli.rb
Expand Up @@ -55,7 +55,8 @@ def test_control_for_tcp
body = s.read
s.close

assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16,"requests_count":0}/, body.split(/\r?\n/).last)
dmt = Puma::Configuration.new.default_max_threads
assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":#{dmt},"max_threads":#{dmt},"requests_count":0}/, body.split(/\r?\n/).last)

ensure
cli.launcher.stop
Expand Down Expand Up @@ -89,7 +90,8 @@ def test_control_for_ssl
body = http.request(req).body
end

expected_stats = /{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16,"requests_count":0}/
dmt = Puma::Configuration.new.default_max_threads
expected_stats = /{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":#{dmt},"max_threads":#{dmt}/
assert_match(expected_stats, body.split(/\r?\n/).last)
assert_equal([:started_at, :backlog, :running, :pool_capacity, :max_threads, :requests_count], Puma.stats.keys)

Expand Down Expand Up @@ -169,7 +171,8 @@ def test_control
body = s.read
s.close

assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16,"requests_count":0}/, body.split("\r\n").last)
dmt = Puma::Configuration.new.default_max_threads
assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":#{dmt},"max_threads":#{dmt},"requests_count":0}/, body.split("\r\n").last)
ensure
if UNIX_SKT_EXIST
cli.launcher.stop
Expand Down
52 changes: 52 additions & 0 deletions test/test_config.rb
Expand Up @@ -9,6 +9,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['PUMA_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['PUMA_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