From 1008c36a4ac1ee77b328f258e128be9853aafa5d Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Sat, 11 Dec 2021 13:58:05 -0800 Subject: [PATCH] Add worker_check_interval configuration option (#2759) Co-authored-by: Olivier Bellone --- lib/puma/cluster.rb | 2 +- lib/puma/cluster/worker.rb | 2 +- lib/puma/configuration.rb | 2 ++ lib/puma/const.rb | 3 --- lib/puma/dsl.rb | 15 ++++++++++++++- test/helpers/integration.rb | 6 ++++++ test/test_integration_cluster.rb | 19 ++++++++++++++++++- test/test_launcher.rb | 2 +- 8 files changed, 43 insertions(+), 8 deletions(-) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index c43e38b4..7d6d1218 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -135,7 +135,7 @@ module Puma def check_workers return if @next_check >= Time.now - @next_check = Time.now + Const::WORKER_CHECK_INTERVAL + @next_check = Time.now + @options[:worker_check_interval] timeout_workers wait_workers diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 07368fd7..9ac3901e 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -130,7 +130,7 @@ module Puma Puma::Util.purge_interrupt_queue break end - sleep Const::WORKER_CHECK_INTERVAL + sleep @options[:worker_check_interval] end end server_thread.join diff --git a/lib/puma/configuration.rb b/lib/puma/configuration.rb index 9d4a2287..974709b0 100644 --- a/lib/puma/configuration.rb +++ b/lib/puma/configuration.rb @@ -11,6 +11,7 @@ module Puma DefaultTCPHost = "0.0.0.0" DefaultTCPPort = 9292 + DefaultWorkerCheckInterval = 5 DefaultWorkerTimeout = 60 DefaultWorkerShutdownTimeout = 30 end @@ -195,6 +196,7 @@ module Puma :workers => Integer(ENV['WEB_CONCURRENCY'] || 0), :silence_single_worker_warning => false, :mode => :http, + :worker_check_interval => DefaultWorkerCheckInterval, :worker_timeout => DefaultWorkerTimeout, :worker_boot_timeout => DefaultWorkerTimeout, :worker_shutdown_timeout => DefaultWorkerShutdownTimeout, diff --git a/lib/puma/const.rb b/lib/puma/const.rb index f2d62028..bbf47b5f 100644 --- a/lib/puma/const.rb +++ b/lib/puma/const.rb @@ -235,9 +235,6 @@ module Puma EARLY_HINTS = "rack.early_hints".freeze - # Minimum interval to checks worker health - WORKER_CHECK_INTERVAL = 5 - # Illegal character in the key or value of response header DQUOTE = "\"".freeze HTTP_HEADER_DELIMITER = Regexp.escape("(),/:;<=>?@[]{}\\").freeze diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index 65b3bbed..308a9687 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -736,6 +736,19 @@ module Puma @options[:tag] = string.to_s end + # Change the default interval for checking workers. + # + # The default value is 5 seconds. + # + # @note Cluster mode only. + # @example + # worker_check_interval 5 + # @see Puma::Cluster#check_workers + # + def worker_check_interval(interval) + @options[:worker_check_interval] = Integer(interval) + end + # Verifies that all workers have checked in to the master process within # the given timeout. If not the worker process will be restarted. This is # not a request timeout, it is to protect against a hung or dead process. @@ -750,7 +763,7 @@ module Puma # def worker_timeout(timeout) timeout = Integer(timeout) - min = Const::WORKER_CHECK_INTERVAL + min = @options.fetch(:worker_check_interval, Puma::ConfigDefault::DefaultWorkerCheckInterval) if timeout <= min raise "The minimum worker_timeout must be greater than the worker reporting interval (#{min})" diff --git a/test/helpers/integration.rb b/test/helpers/integration.rb index 856167a0..0326750a 100644 --- a/test/helpers/integration.rb +++ b/test/helpers/integration.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "puma/control_cli" +require "json" require "open3" require "io/wait" require_relative 'tmp_path' @@ -255,6 +256,11 @@ class TestIntegration < Minitest::Test r end + def get_stats + read_pipe = cli_pumactl "stats" + JSON.parse(read_pipe.readlines.last) + end + def hot_restart_does_not_drop_connections(num_threads: 1, total_requests: 500) skipped = true skip_if :jruby, suffix: <<-MSG diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index 63e1b193..b5d8abac 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -1,6 +1,8 @@ require_relative "helper" require_relative "helpers/integration" +require "time" + class TestIntegrationCluster < TestIntegration parallelize_me! if ::Puma.mri? @@ -147,6 +149,21 @@ class TestIntegrationCluster < TestIntegration worker_respawn { |phase0_worker_pids| Process.kill :USR1, @pid } end + def test_worker_check_interval + @control_tcp_port = UniquePort.call + worker_check_interval = 1 + + cli_server "-w 1 -t 1:1 --control-url tcp://#{HOST}:#{@control_tcp_port} --control-token #{TOKEN} test/rackup/hello.ru", config: "worker_check_interval #{worker_check_interval}" + + sleep worker_check_interval + 1 + last_checkin_1 = Time.parse(get_stats["worker_status"].first["last_checkin"]) + + sleep worker_check_interval + 1 + last_checkin_2 = Time.parse(get_stats["worker_status"].first["last_checkin"]) + + assert(last_checkin_2 > last_checkin_1) + end + def test_worker_boot_timeout timeout = 1 worker_timeout(timeout, 2, "worker failed to boot within \\\d+ seconds", "worker_boot_timeout #{timeout}; on_worker_boot { sleep #{timeout + 1} }") @@ -154,7 +171,7 @@ class TestIntegrationCluster < TestIntegration def test_worker_timeout skip 'Thread#name not available' unless Thread.current.respond_to?(:name) - timeout = Puma::Const::WORKER_CHECK_INTERVAL + 1 + timeout = Puma::ConfigDefault::DefaultWorkerCheckInterval + 1 worker_timeout(timeout, 1, "worker failed to check in within \\\d+ seconds", <