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

Warn when clustered-only hooks are defined in single mode #3089

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions lib/puma/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class Configuration
reaping_time: 1,
remote_address: :socket,
silence_single_worker_warning: false,
silence_fork_callback_warning: false,
tag: File.basename(Dir.getwd),
tcp_host: '0.0.0.0'.freeze,
tcp_port: 9292,
Expand Down
30 changes: 29 additions & 1 deletion lib/puma/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,11 @@ def silence_single_worker_warning
@options[:silence_single_worker_warning] = true
end

# Disable warning message when running single mode with callback hook defined.
def silence_fork_callback_warning
@options[:silence_fork_callback_warning] = true
end

# Code to run immediately before master process
# forks workers (once on boot). These hooks can block if necessary
# to wait for background operations unknown to Puma to finish before
Expand All @@ -600,6 +605,8 @@ def silence_single_worker_warning
# puts "Starting workers..."
# end
def before_fork(&block)
warn_if_in_single_mode('before_fork')

@options[:before_fork] ||= []
@options[:before_fork] << block
end
Expand All @@ -615,6 +622,8 @@ def before_fork(&block)
# puts 'Before worker boot...'
# end
def on_worker_boot(key = nil, &block)
warn_if_in_single_mode('on_worker_boot')

process_hook :before_worker_boot, key, block, 'on_worker_boot'
end

Expand All @@ -631,6 +640,8 @@ def on_worker_boot(key = nil, &block)
# puts 'On worker shutdown...'
# end
def on_worker_shutdown(key = nil, &block)
warn_if_in_single_mode('on_worker_shutdown')

process_hook :before_worker_shutdown, key, block, 'on_worker_shutdown'
end

Expand All @@ -645,6 +656,8 @@ def on_worker_shutdown(key = nil, &block)
# puts 'Before worker fork...'
# end
def on_worker_fork(&block)
warn_if_in_single_mode('on_worker_fork')

process_hook :before_worker_fork, nil, block, 'on_worker_fork'
end

Expand All @@ -659,6 +672,8 @@ def on_worker_fork(&block)
# puts 'After worker fork...'
# end
def after_worker_fork(&block)
warn_if_in_single_mode('after_worker_fork')

process_hook :after_worker_fork, nil, block, 'after_worker_fork'
end

Expand Down Expand Up @@ -1073,7 +1088,20 @@ def process_hook(options_key, key, block, meth)
elsif key.nil?
@options[options_key] << block
else
raise "'#{method}' key must be String or Symbol"
raise "'#{meth}' key must be String or Symbol"
Vuta marked this conversation as resolved.
Show resolved Hide resolved
end
end

def warn_if_in_single_mode(hook_name)
return if @options[:silence_fork_callback_warning]

if (@options[:workers] || 0) == 0
log_string =
"Warning: You specified code to run in a `#{hook_name}` block, " \
"but Puma is configured to run in cluster mode, " \
Copy link
Contributor

Choose a reason for hiding this comment

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

"so your `#{hook_name}` block did not run"

LogWriter.stdio.log(log_string)
end
end
end
Expand Down
42 changes: 41 additions & 1 deletion test/test_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -455,22 +455,32 @@ def test_run_hooks_on_restart_hook

def test_run_hooks_before_worker_fork
assert_run_hooks :before_worker_fork, configured_with: :on_worker_fork

assert_warning_for_hooks_defined_in_single_mode :on_worker_fork
end

def test_run_hooks_after_worker_fork
assert_run_hooks :after_worker_fork

assert_warning_for_hooks_defined_in_single_mode :after_worker_fork
end

def test_run_hooks_before_worker_boot
assert_run_hooks :before_worker_boot, configured_with: :on_worker_boot

assert_warning_for_hooks_defined_in_single_mode :on_worker_boot
end

def test_run_hooks_before_worker_shutdown
assert_run_hooks :before_worker_shutdown, configured_with: :on_worker_shutdown

assert_warning_for_hooks_defined_in_single_mode :on_worker_shutdown
end

def test_run_hooks_before_fork
assert_run_hooks :before_fork

assert_warning_for_hooks_defined_in_single_mode :before_fork
end

def test_run_hooks_and_exception
Expand Down Expand Up @@ -514,6 +524,22 @@ def test_silence_single_worker_warning_overwrite
assert_equal true, conf.options[:silence_single_worker_warning]
end

def test_silence_fork_callback_warning_default
conf = Puma::Configuration.new
conf.load

assert_equal false, conf.options[:silence_fork_callback_warning]
end

def test_silence_fork_callback_warning_overwrite
conf = Puma::Configuration.new do |c|
c.silence_fork_callback_warning
end
conf.load

assert_equal true, conf.options[:silence_fork_callback_warning]
end

def test_http_content_length_limit
assert_nil Puma::Configuration.new.options.default_options[:http_content_length_limit]

Expand All @@ -529,7 +555,9 @@ def assert_run_hooks(hook_name, options = {})

# test single, not an array
messages = []
conf = Puma::Configuration.new
conf = Puma::Configuration.new do |c|
c.silence_fork_callback_warning
end
conf.options[hook_name] = -> (a) {
messages << "#{hook_name} is called with #{a}"
}
Expand All @@ -540,6 +568,8 @@ def assert_run_hooks(hook_name, options = {})
# test multiple
messages = []
conf = Puma::Configuration.new do |c|
c.silence_fork_callback_warning
Vuta marked this conversation as resolved.
Show resolved Hide resolved

c.send(configured_with) do |a|
messages << "#{hook_name} is called with #{a} one time"
end
Expand All @@ -553,6 +583,16 @@ def assert_run_hooks(hook_name, options = {})
conf.run_hooks(hook_name, 'ARG', Puma::LogWriter.strings)
assert_equal messages, ["#{hook_name} is called with ARG one time", "#{hook_name} is called with ARG a second time"]
end

def assert_warning_for_hooks_defined_in_single_mode(hook_name)
out, _ = capture_io do
conf = Puma::Configuration.new do |c|
c.send(hook_name)
end
end

assert_match "your `#{hook_name}` block did not run\n", out
end
end

# Thread unsafe modification of ENV
Expand Down