From b2e8adf140e0c5cda56a51ec916ccda9938135be Mon Sep 17 00:00:00 2001 From: shields Date: Sun, 9 Jan 2022 20:04:23 +0900 Subject: [PATCH 1/4] When using fork_worker mode, worker_culling_strategy "oldest" shouldn't cull worker 0. --- lib/puma/cluster.rb | 21 ++++++++++++--------- test/test_integration_cluster.rb | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index d90c81d569..749704aea0 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -109,15 +109,9 @@ def cull_workers diff = @workers.size - @options[:workers] return if diff < 1 - debug "Culling #{diff.inspect} workers" - - workers_to_cull = - case @options[:worker_culling_strategy] - when :youngest - @workers.sort_by(&:started_at)[-diff,diff] - when :oldest - @workers.sort_by(&:started_at)[0,diff] - end + debug "Culling #{diff} workers" + + workers_to_cull = @workers.sort_by(&:started_at)[cull_start_index(diff), diff] debug "Workers to cull: #{workers_to_cull.inspect}" @@ -127,6 +121,15 @@ def cull_workers end end + def cull_start_index(diff) + case @options[:worker_culling_strategy] + when :oldest + @options[:fork_worker] ? 1 : 0 + else # :youngest + -diff + end + end + # @!attribute [r] next_worker_index def next_worker_index occupied_positions = @workers.map(&:index) diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index aaccd38507..979765f067 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -401,6 +401,25 @@ def test_culling_strategy_oldest assert_match(/Worker 0 \(PID: \d+\) terminating/, line) end + def test_culling_strategy_oldest_fork_worker + cli_server "-w 2 test/rackup/hello.ru", config: < Date: Sun, 9 Jan 2022 20:13:29 +0900 Subject: [PATCH 2/4] Consider case where worker0 isn't oldest worker --- lib/puma/cluster.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 749704aea0..c2afe20ba6 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -111,7 +111,9 @@ def cull_workers debug "Culling #{diff} workers" - workers_to_cull = @workers.sort_by(&:started_at)[cull_start_index(diff), diff] + workers_to_cull = @workers.sort_by(&:started_at) + workers_to_cull.reject! { |w| w.index == 0 } if @options[:fork_worker] + workers_to_cull = workers_to_cull[cull_start_index(diff), diff] debug "Workers to cull: #{workers_to_cull.inspect}" @@ -124,7 +126,7 @@ def cull_workers def cull_start_index(diff) case @options[:worker_culling_strategy] when :oldest - @options[:fork_worker] ? 1 : 0 + 0 else # :youngest -diff end From 26330ce773a28e46cbe50b8451ce50c4b66082ac Mon Sep 17 00:00:00 2001 From: shields Date: Sun, 9 Jan 2022 20:18:58 +0900 Subject: [PATCH 3/4] Reduce code complexity --- lib/puma/cluster.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index c2afe20ba6..567398db11 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -108,21 +108,23 @@ def spawn_worker(idx, master) def cull_workers diff = @workers.size - @options[:workers] return if diff < 1 - debug "Culling #{diff} workers" - workers_to_cull = @workers.sort_by(&:started_at) - workers_to_cull.reject! { |w| w.index == 0 } if @options[:fork_worker] - workers_to_cull = workers_to_cull[cull_start_index(diff), diff] - - debug "Workers to cull: #{workers_to_cull.inspect}" + workers = workers_to_cull(diff) + debug "Workers to cull: #{workers.inspect}" - workers_to_cull.each do |worker| + workers.each do |worker| log "- Worker #{worker.index} (PID: #{worker.pid}) terminating" worker.term end end + def workers_to_cull(diff) + workers = @workers.sort_by(&:started_at) + workers.reject! { |w| w.index == 0 } if @options[:fork_worker] + workers[cull_start_index(diff), diff] + end + def cull_start_index(diff) case @options[:worker_culling_strategy] when :oldest From e409a93324813b09cb4b8cfb913803266ee8877b Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Mon, 10 Jan 2022 13:46:50 +0900 Subject: [PATCH 4/4] Add code comment --- lib/puma/cluster.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 567398db11..5c3303a428 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -121,7 +121,11 @@ def cull_workers def workers_to_cull(diff) workers = @workers.sort_by(&:started_at) + + # In fork_worker mode, worker 0 acts as our master process. + # We should avoid culling it to preserve copy-on-write memory gains. workers.reject! { |w| w.index == 0 } if @options[:fork_worker] + workers[cull_start_index(diff), diff] end