Skip to content

Commit

Permalink
Closes #2950 - Warn when clustered-only hooks are defined in single mode
Browse files Browse the repository at this point in the history
Clustered-only hooks won't be called in single mode, so this commit adds a log to warn users about that.
Also adds `silence_fork_callback_warning` option to opt-out of the default warning log.
  • Loading branch information
Vuta committed Mar 1, 2023
1 parent 0fd1cc5 commit 5d8afd4
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 2 deletions.
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"
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, " \
"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

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

0 comments on commit 5d8afd4

Please sign in to comment.