From 53839f96815788fe76b9727de79c930bda1006e8 Mon Sep 17 00:00:00 2001 From: Will Jordan Date: Thu, 13 Feb 2020 13:26:27 -0800 Subject: [PATCH] Add fork_worker option and refork command Trigger refork for improved copy-on-write performance. --- History.md | 1 + lib/puma/app/status.rb | 5 +++ lib/puma/cli.rb | 5 +++ lib/puma/cluster.rb | 73 +++++++++++++++++++++++++++----- lib/puma/control_cli.rb | 5 ++- lib/puma/dsl.rb | 12 ++++++ test/test_integration_cluster.rb | 9 +++- 7 files changed, 97 insertions(+), 13 deletions(-) diff --git a/History.md b/History.md index 9a38780c9a..8c0e34cd68 100644 --- a/History.md +++ b/History.md @@ -7,6 +7,7 @@ * `Puma.stats` now returns a Hash instead of a JSON string (#2086) * `GC.compact` is called before fork if available (#2093) * Add `requests_count` to workers stats. (#2106) + * Add `fork_worker` option and `refork` command for improved copy-on-write performance (#2099) * Bugfixes * Your bugfix goes here (#Github Number) diff --git a/lib/puma/app/status.rb b/lib/puma/app/status.rb index 3a6518ee78..efffc43cff 100644 --- a/lib/puma/app/status.rb +++ b/lib/puma/app/status.rb @@ -63,6 +63,11 @@ def call(env) end rack_response(200, backtraces.to_json) + + when /\/refork$/ + Process.kill "SIGURG", $$ + rack_response(200, OK_STATUS) + else rack_response 404, "Unsupported action", 'text/plain' end diff --git a/lib/puma/cli.rb b/lib/puma/cli.rb index cff30af861..bc116d5698 100644 --- a/lib/puma/cli.rb +++ b/lib/puma/cli.rb @@ -140,6 +140,11 @@ def setup_options user_config.environment arg end + o.on "-f", "--fork-worker", + "Fork new workers from existing worker. Cluster mode only" do + user_config.fork_worker + end + o.on "-I", "--include PATH", "Specify $LOAD_PATH directories" do |arg| $LOAD_PATH.unshift(*arg.split(':')) end diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 989828fb33..132ebd9637 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -78,6 +78,7 @@ def initialize(idx, pid, phase, options) end attr_reader :index, :pid, :phase, :signal, :last_checkin, :last_status, :started_at + attr_writer :pid, :phase def booted? @stage == :booted @@ -113,7 +114,7 @@ def term @term ||= true @first_term_sent ||= Time.now end - Process.kill @signal, @pid + Process.kill @signal, @pid if @pid rescue Errno::ESRCH end end @@ -139,11 +140,11 @@ def spawn_workers idx = next_worker_index @launcher.config.run_hooks :before_worker_fork, idx - pid = fork { worker(idx, master) } - if !pid - log "! Complete inability to spawn new workers detected" - log "! Seppuku is the only choice." - exit! 1 + if @options[:fork_worker] && idx != 0 + @fork_writer << "#{idx}\n" + pid = nil + else + pid = spawn_worker(idx, master) end debug "Spawned worker: #{pid}" @@ -157,6 +158,16 @@ def spawn_workers end end + def spawn_worker(idx, master) + pid = fork { worker(idx, master) } + if !pid + log "! Complete inability to spawn new workers detected" + log "! Seppuku is the only choice." + exit! 1 + end + pid + end + def cull_workers diff = @workers.size - @options[:workers] return if diff < 1 @@ -246,8 +257,11 @@ def worker(index, master) Signal.trap "SIGINT", "IGNORE" @workers = [] - @master_read.close - @suicide_pipe.close + if !@options[:fork_worker] || index == 0 + @master_read.close + @suicide_pipe.close + @fork_writer.close + end Thread.new do Puma.set_thread_name "worker check pipe" @@ -272,13 +286,29 @@ def worker(index, master) server = start_server + if index == 0 + Signal.trap "SIGCHLD" do + Process.wait(-1, Process::WNOHANG) rescue nil + wakeup! + end + + Thread.new do + Puma.set_thread_name "worker fork pipe" + while (idx = @fork_pipe.gets) + idx = idx.to_i + pid = spawn_worker(idx, master) + @worker_write << "f#{pid}:#{idx}\n" rescue nil + end + end + end + Signal.trap "SIGTERM" do @worker_write << "e#{Process.pid}\n" rescue nil server.stop end begin - @worker_write << "b#{Process.pid}\n" + @worker_write << "b#{Process.pid}:#{index}\n" rescue SystemCallError, IOError Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue STDERR.puts "Master seems to have exited, exiting." @@ -386,6 +416,13 @@ def preload? # We do this in a separate method to keep the lambda scope # of the signals handlers as small as possible. def setup_signals + Signal.trap "SIGURG" do + if (worker = @workers.find {|w| w.index == 0}) + worker.phase += 1 + end + phased_restart + end + Signal.trap "SIGCHLD" do wakeup! end @@ -469,6 +506,10 @@ def run # @check_pipe, @suicide_pipe = Puma::Util.pipe + # Separate pipe used by worker 0 to receive commands to + # fork new worker processes. + @fork_pipe, @fork_writer = Puma::Util.pipe + if daemon? log "* Daemonizing..." Process.daemon(true) @@ -521,6 +562,12 @@ def run result = read.gets pid = result.to_i + if req == "b" || req == "f" + pid, idx = result.split(':').map(&:to_i) + w = @workers.find {|x| x.index == idx} + w.pid = pid if w.pid.nil? + end + if w = @workers.find { |x| x.pid == pid } case req when "b" @@ -561,6 +608,7 @@ def run # `#term` if needed def wait_workers @workers.reject! do |w| + next false if w.pid.nil? begin if Process.wait(w.pid, Process::WNOHANG) true @@ -569,7 +617,12 @@ def wait_workers nil end rescue Errno::ECHILD - true # child is already terminated + begin + Process.kill(0, w.pid) + false # child still alive, but has another parent + rescue Errno::ESRCH, Errno::EPERM + true # child is already terminated + end end end end diff --git a/lib/puma/control_cli.rb b/lib/puma/control_cli.rb index 2897f63063..730b5e7fdd 100644 --- a/lib/puma/control_cli.rb +++ b/lib/puma/control_cli.rb @@ -11,7 +11,7 @@ module Puma class ControlCLI - COMMANDS = %w{halt restart phased-restart start stats status stop reload-worker-directory gc gc-stats thread-backtraces} + COMMANDS = %w{halt restart phased-restart start stats status stop reload-worker-directory gc gc-stats thread-backtraces refork} PRINTABLE_COMMANDS = %w{gc-stats stats thread-backtraces} def initialize(argv, stdout=STDOUT, stderr=STDERR) @@ -232,6 +232,9 @@ def send_signal return + when "refork" + Process.kill "SIGURG", @pid + else return end diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index d2d97620f3..3382ccc69f 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -736,5 +736,17 @@ def set_remote_address(val=:socket) end end + # When enabled, workers will be forked from worker 0 instead of from the master process. + # This enables the `refork` command to optimize copy-on-write performance in a running app. + # + # This option is similar to `preload_app` because the app is preloaded before forking, + # but it is compatible with phased restart. + # + # `preload_app` is not required when this option is enabled. + # + # @note Cluster mode only. + def fork_worker(answer=true) + @options[:fork_worker] = answer + end end end diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index a7f58f84ec..1a3aa3d1e7 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -75,6 +75,11 @@ def test_usr1_all_respond_tcp usr1_all_respond unix: false end + def test_usr1_fork_worker + skip_unless_signal_exist? :USR1 + usr1_all_respond config: '--fork-worker' + end + def test_usr1_all_respond_unix skip_unless_signal_exist? :USR1 usr1_all_respond unix: true @@ -203,8 +208,8 @@ def term_closes_listeners(unix: false) # Send requests 1 per second. Send 1, then :USR1 server, then send another 24. # All should be responded to, and at least three workers should be used - def usr1_all_respond(unix: false) - cli_server "-w #{WORKERS} -t 0:5 -q test/rackup/sleep_pid.ru", unix: unix + def usr1_all_respond(unix: false, config: '') + cli_server "-w #{WORKERS} -t 0:5 -q test/rackup/sleep_pid.ru #{config}", unix: unix threads = [] replies = [] mutex = Mutex.new