From c9256e644ca0ebb5904747e70e0edd1181b614a4 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 4 Oct 2021 14:42:37 +0100 Subject: [PATCH] only use puma, introduce rack-timeout (#9718) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Oliver Günther --- Gemfile | 11 ++-- Gemfile.lock | 19 +++---- config.ru | 17 ------ config/configuration.yml.example | 13 +++++ config/initializers/database_pool_size.rb | 9 ++++ config/initializers/rack_timeout.rb | 43 +++++++++++++++ config/puma.rb | 25 +++++++-- config/unicorn.rb | 54 ------------------- .../configuration/README.md | 50 +++++++++++++++++ .../packaged/openproject-apache-example.conf | 2 +- lib/open_project/configuration.rb | 16 +++++- lib/open_project/configuration/helpers.rb | 28 ++++++++++ nix/gemset.nix | 22 -------- packaging/scripts/check | 2 +- packaging/scripts/web | 10 +--- 15 files changed, 192 insertions(+), 129 deletions(-) create mode 100644 config/initializers/database_pool_size.rb create mode 100644 config/initializers/rack_timeout.rb delete mode 100644 config/unicorn.rb diff --git a/Gemfile b/Gemfile index 37b74e05ed4..880a2a933f1 100644 --- a/Gemfile +++ b/Gemfile @@ -156,20 +156,15 @@ group :production do # we use dalli as standard memcache client # requires memcached 1.4+ gem 'dalli', '~> 2.7.10' - - # Unicorn worker killer to restart unicorn child workers - gem 'unicorn-worker-killer', require: false end gem 'i18n-js', '~> 3.9.0' gem 'rails-i18n', '~> 6.0.0' gem 'sprockets', '~> 3.7.0' -# required by Procfile, for deployment on heroku or packaging with packager.io. -# also, better than thin since we can control worker concurrency. -gem 'unicorn' - -gem 'puma', '~> 5.5.0' # used for development and optionally for production +gem 'puma', '~> 5.5' +gem 'rack-timeout', '~> 0.6.0', require: "rack/timeout/base" +gem 'puma-plugin-statsd', '~> 2.0' gem 'nokogiri', '~> 1.12.5' diff --git a/Gemfile.lock b/Gemfile.lock index 56ff84cd0f5..f0aa8659787 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -510,8 +510,6 @@ GEM fuubar (2.5.1) rspec-core (~> 3.0) ruby-progressbar (~> 1.4) - get_process_mem (0.2.7) - ffi (~> 1.0) git (1.9.1) rchardet (~> 1.8) globalid (0.5.2) @@ -569,7 +567,6 @@ GEM json_spec (1.1.5) multi_json (~> 1.0) rspec (>= 2.0, < 4.0) - kgio (2.11.4) kramdown (2.3.1) rexml kramdown-parser-gfm (1.1.0) @@ -707,6 +704,8 @@ GEM multi_json puma (5.5.0) nio4r (~> 2.0) + puma-plugin-statsd (2.0.0) + puma (>= 5.0, < 6) raabro (1.4.0) racc (1.5.2) rack (2.2.3) @@ -728,6 +727,7 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) + rack-timeout (0.6.0) rack_session_access (0.2.0) builder (>= 2.0.0) rack (>= 1.0.0) @@ -765,7 +765,6 @@ GEM rake (>= 0.13) thor (~> 1.0) rainbow (3.0.0) - raindrops (0.19.2) rake (13.0.6) rb-fsevent (0.11.0) rb-inotify (0.10.1) @@ -932,12 +931,6 @@ GEM unf_ext unf_ext (0.0.8) unicode-display_width (2.1.0) - unicorn (6.0.0) - kgio (~> 2.6) - raindrops (~> 0.7) - unicorn-worker-killer (0.4.5) - get_process_mem (~> 0) - unicorn (>= 4, < 7) uri_template (0.7.0) validate_email (0.1.6) activemodel (>= 3.0) @@ -1081,12 +1074,14 @@ DEPENDENCIES pry-rescue (~> 1.5.2) pry-stack_explorer (~> 0.6.0) puffing-billy (~> 2.4.0) - puma (~> 5.5.0) + puma (~> 5.5) + puma-plugin-statsd (~> 2.0) rack-attack (~> 6.5.0) rack-cors (~> 1.1.1) rack-mini-profiler rack-protection (~> 2.1.0) rack-test (~> 1.1.0) + rack-timeout (~> 0.6.0) rack_session_access rails (~> 6.1.3) rails-controller-testing (~> 1.0.2) @@ -1132,8 +1127,6 @@ DEPENDENCIES timecop (~> 0.9.0) typed_dag (~> 2.0.2) tzinfo-data (~> 1.2021.1) - unicorn - unicorn-worker-killer warden (~> 1.2) warden-basic_auth (~> 0.2.1) webdrivers (~> 4.6.0) diff --git a/config.ru b/config.ru index 3d80cbd289c..b245154bb58 100644 --- a/config.ru +++ b/config.ru @@ -32,23 +32,6 @@ require ::File.expand_path('config/environment', __dir__) -## -# Use the worker killer when Unicorn is being used -if defined?(Unicorn) && Rails.env.production? - require 'unicorn/worker_killer' - - min_ram = ENV.fetch('OPENPROJECT_UNICORN_RAM2KILL_MIN', 340 * 1 << 20).to_i - max_ram = ENV.fetch('OPENPROJECT_UNICORN_RAM2KILL_MAX', 400 * 1 << 20).to_i - min_req = ENV.fetch('OPENPROJECT_UNICORN_REQ2KILL_MIN', 3072).to_i - max_req = ENV.fetch('OPENPROJECT_UNICORN_REQ2KILL_MAX', 4096).to_i - - # Kill Workers randomly between 340 and 400 MB (per default) - # or between 3072 and 4096 requests. - # Our largest installations are starting around 200/230 MB - use Unicorn::WorkerKiller::Oom, min_ram, max_ram - use Unicorn::WorkerKiller::MaxRequests, min_req, max_req -end - subdir = OpenProject::Configuration.rails_relative_url_root.presence map (subdir || '/') do diff --git a/config/configuration.yml.example b/config/configuration.yml.example index 3bda1bafd49..0c9a3d58c75 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -167,6 +167,19 @@ default: log_level: info + # web server configuration + # web: + # workers: 2 + # timeout: 60 + # wait_timeout: 10 + # min_threads: 4 + # max_threads: 16 + + # statsd configuration + # statsd: + # host: 127.0.0.1 + # port: 8125 + # Outgoing emails configuration (see examples above) email_delivery_method: :smtp smtp_address: smtp.example.net diff --git a/config/initializers/database_pool_size.rb b/config/initializers/database_pool_size.rb new file mode 100644 index 00000000000..2e40302a69e --- /dev/null +++ b/config/initializers/database_pool_size.rb @@ -0,0 +1,9 @@ +config = Rails.application.config.database_configuration[Rails.env] +pool_size = [OpenProject::Configuration.web_max_threads + 1, config['pool'].to_i].max + +# make sure we have enough connections in the pool for each thread and then some +if Rails.env.production? && pool_size > ActiveRecord::Base.connection_pool.size + Rails.logger.debug { "Increasing database pool size to #{pool_size} to match max threads" } + + ActiveRecord::Base.establish_connection config.merge(pool: pool_size) +end diff --git a/config/initializers/rack_timeout.rb b/config/initializers/rack_timeout.rb new file mode 100644 index 00000000000..40095d346b8 --- /dev/null +++ b/config/initializers/rack_timeout.rb @@ -0,0 +1,43 @@ +# Use rack-timeout if we run in clustered mode with at least 2 workers +# so that workers, should a timeout occur, can be restarted without interruption. +if OpenProject::Configuration.web_workers >= 2 + timeout = Integer(ENV['RACK_TIMEOUT_SERVICE_TIMEOUT'].presence || OpenProject::Configuration.web_timeout) + wait_timeout = Integer(ENV['RACK_TIMEOUT_WAIT_TIMEOUT'].presence || OpenProject::Configuration.web_wait_timeout) + + Rails.logger.debug { "Enabling Rack::Timeout (service=#{timeout}s wait=#{wait_timeout}s)" } + + Rails.application.config.middleware.insert_before( + ::Rack::Runtime, + ::Rack::Timeout, + service_timeout: timeout, # time after which a request being served times out + wait_timeout: wait_timeout, # time after which a request waiting to be served times out + term_on_timeout: 1 # shut down worker (gracefully) right away on timeout to be restarted + ) + + # remove default logger (logging uninteresting extra info with each not timed out request) + Rack::Timeout.unregister_state_change_observer(:logger) + + Rack::Timeout.register_state_change_observer(:wait_timeout_logger) do |env| + details = env[Rack::Timeout::ENV_INFO_KEY] + + if details.state == :timed_out && details.wait.present? + ::OpenProject.logger.error "Request timed out waiting to be served!" + end + end + + # The timeout itself is already reported so no need to + # report the generic internal server error too as it doesn't + # add any more information. Even worse, it's not immediately + # clear that the two reports are related. + module SuppressInternalErrorReportOnTimeout + def op_handle_error(message_or_exception, context = {}) + return if request && request.env[Rack::Timeout::ENV_INFO_KEY].try(:state) == :timed_out + + super + end + end + + OpenProjectErrorHelper.prepend SuppressInternalErrorReportOnTimeout +else + Rails.logger.debug { "Not enabling Rack::Timeout since we are not running in cluster mode with at least 2 workers" } +end diff --git a/config/puma.rb b/config/puma.rb index 3c434920b5e..54fd04f2e6a 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -3,8 +3,8 @@ # Any libraries that use thread pools should be configured to match # the maximum value specified for Puma. # -threads_min_count = ENV.fetch("RAILS_MIN_THREADS") { 4 }.to_i -threads_max_count = ENV.fetch("RAILS_MAX_THREADS") { 16 }.to_i +threads_min_count = OpenProject::Configuration.web_min_threads +threads_max_count = OpenProject::Configuration.web_max_threads threads threads_min_count, [threads_min_count, threads_max_count].max # Specifies the `port` that Puma will listen on to receive requests; default is 3000. @@ -21,7 +21,7 @@ environment ENV.fetch("RAILS_ENV") { "development" } # Workers do not work on JRuby or Windows (both of which do not support # processes). # -workers ENV.fetch("OPENPROJECT_WEB_WORKERS") { 0 }.to_i +workers OpenProject::Configuration.web_workers # Use the `preload_app!` method when specifying a `workers` number. # This directive tells Puma to first boot the application and load code @@ -32,3 +32,22 @@ preload_app! if ENV["RAILS_ENV"] == 'production' # Allow puma to be restarted by `rails restart` command. plugin :tmp_restart unless ENV["RAILS_ENV"] == 'production' + +# activate statsd plugin only if a host is configured explicitly +if OpenProject::Configuration.statsd_host.present? + module ConfigurationViaOpenProject + def initialize + host = OpenProject::Configuration.statsd_host + port = OpenProject::Configuration.statsd_port + + Rails.logger.debug { "Enabling puma statsd plugin (publish to udp://#{host}:#{port})" } + + @host = host + @port = port + end + end + + StatsdConnector.prepend ConfigurationViaOpenProject + + plugin :statsd +end diff --git a/config/unicorn.rb b/config/unicorn.rb deleted file mode 100644 index 1274eabe41d..00000000000 --- a/config/unicorn.rb +++ /dev/null @@ -1,54 +0,0 @@ -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2012-2021 the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -worker_processes Integer(ENV['OPENPROJECT_WEB_WORKERS'] || 4) -timeout Integer(ENV['OPENPROJECT_WEB_TIMEOUT'] || 300) -preload_app true - -# Preloading the unicorn server to have all workers spawn the application -# automatically. -# -# Borrows heavily from https://www.digitalocean.com/community/tutorials/how-to-optimize-unicorn-workers-in-a-ruby-on-rails-app -# -# This method requires ActiveRecord to close and re-establish its connection in the slaves, -# because the connection is not properly shared with them. -# -# If you use any other service, you'll need to add them to these _fork blocks to close -# and reopen sockets when forking. -# (except Dalli/Memcache store, which detects automatically) -before_fork do |_server, _worker| - if defined?(ActiveRecord::Base) - ActiveRecord::Base.connection.disconnect! - end -end - -after_fork do |_server, _worker| - if defined?(ActiveRecord::Base) - ActiveRecord::Base.establish_connection - end -end diff --git a/docs/installation-and-operations/configuration/README.md b/docs/installation-and-operations/configuration/README.md index a48bbdc4bd1..701840845e0 100644 --- a/docs/installation-and-operations/configuration/README.md +++ b/docs/installation-and-operations/configuration/README.md @@ -48,6 +48,8 @@ Configuring OpenProject through environment variables is detailed [in this separ * [`enterprise_limits`](#enterprise-limits) * [`backup_enabled`](#backup-enabled) * [`show_community_links`](#show-community-links) +* [`web`](#web) (nested configuration) +* [`statsd`](#statsd) (nested configuration) ## Setting session options @@ -425,6 +427,54 @@ If you would like to hide the homescreen links to the OpenProject community, you OPENPROJECT_SHOW__COMMUNITY__LINKS=false ``` +### Web + +Configuration of the main ruby web server (currently puma). Sensible defaults are provided. + +``` +web: + workers: 2 # number of server processes + timeout: 60 # seconds before a request times out + wait_timeout: 10 # seconds before a request waiting to be served times out + min_threads: 4 + max_threads: 16 +``` + +**Note:** Timeouts only are supported when using at least 2 workers. + +As usual these values can be overriden via the environment. + +``` +OPENPROJECT_WEB_WORKERs=2 +OPENPROJECT_WEB_TIMEOUT=60 # overriden by: RACK_TIMEOUT_SERVICE_TIMEOUT +OPENPROJECT_WEB_WAIT__TIMEOUT=10 # overriden by: RACK_TIMEOUT_WAIT_TIMEOUT +OPENPROJECT_WEB_MIN__THREADS=4 # overriden by: RAILS_MIN_THREADS +OPENPROJECT_WEB_MAX__THREADS=16 # overriden by: RAILS_MAX_THREADS +``` + +### statsd + +*default: { host: nil, port: 8125 }* + +OpenProject can push metrics to [statsd](https://github.com/statsd/statsd). +Currently these are simply the metrics for the puma server +but this may include more in the future. + +This is disabled by default unless a host configured. + +``` +statsd: + host: 127.0.0.1 + port: 8125 +``` + +Or via the environment: + +``` +OPENPROJECT_STATSD_HOST=127.0.0.1 # overriden by: STATSD_HOST +OPENPRJOECT_STATSD_PORT=8125 # overriden by: STATSD_PORT +``` + | ----------- | :---------- | | [List of supported environment variables](./environment) | The full list of environment variables you can use to override the default configuration | | [Configuring SSL](./ssl) | How to configure SSL so that your OpenProject installation is available over HTTPS | diff --git a/docs/installation-and-operations/installation/packaged/openproject-apache-example.conf b/docs/installation-and-operations/installation/packaged/openproject-apache-example.conf index cc0a5ef93fc..e6d14d1b315 100644 --- a/docs/installation-and-operations/installation/packaged/openproject-apache-example.conf +++ b/docs/installation-and-operations/installation/packaged/openproject-apache-example.conf @@ -26,7 +26,7 @@ ServerAdmin admin@example.com DocumentRoot /opt/openproject/public - # Proxy requests to localhost:6000 / unicorn worker + # Proxy requests to localhost:6000 (puma) ProxyRequests off ProxyPass / http://127.0.0.1:6000/ retry=0 ProxyPassReverse / http://127.0.0.1:6000/ diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index a8bf47ea4d1..142434f4567 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -190,6 +190,12 @@ module OpenProject # set to n >= 1 to enable n times the default tracing 'sentry_frontend_trace_factor' => 0, + # enable statsd metrics (currently puma only) by configuring host + 'statsd' => { + 'host' => nil, + 'port' => 8125 + }, + # Allow connections for trial creation and booking 'enterprise_trial_creation_host' => 'https://augur.openproject.com', 'enterprise_chargebee_site' => 'openproject-enterprise', @@ -207,7 +213,15 @@ module OpenProject 'sql_slow_query_threshold' => 2000, # Use lograge to format logs, off by default - 'lograge_formatter' => nil + 'lograge_formatter' => nil, + + 'web' => { + 'workers' => 2, + 'timeout' => 120, + 'wait_timeout' => 10, + 'min_threads' => 4, + 'max_threads' => 16 + } } @config = nil diff --git a/lib/open_project/configuration/helpers.rb b/lib/open_project/configuration/helpers.rb index 18b751fbf8f..b5beb22fc76 100644 --- a/lib/open_project/configuration/helpers.rb +++ b/lib/open_project/configuration/helpers.rb @@ -170,6 +170,34 @@ module OpenProject val.presence || {} end + def web_workers + Integer(web['workers'].presence) + end + + def web_timeout + Integer(web['timeout'].presence) + end + + def web_wait_timeout + Integer(web['wait_timeout'].presence) + end + + def web_min_threads + Integer(ENV['RAILS_MIN_THREADS'].presence || web['min_threads'].presence) + end + + def web_max_threads + Integer(ENV['RAILS_MAX_THREADS'].presence || web['max_threads'].presence) + end + + def statsd_host + ENV['STATSD_HOST'].presence || statsd['host'].presence + end + + def statsd_port + Integer(ENV['STATSD_PORT'].presence || statsd['port'].presence) + end + private ## diff --git a/nix/gemset.nix b/nix/gemset.nix index a74f0adc8cb..eb3296d526b 100644 --- a/nix/gemset.nix +++ b/nix/gemset.nix @@ -3596,28 +3596,6 @@ }; version = "2.0.0"; }; - unicorn = { - dependencies = ["kgio" "raindrops"]; - groups = ["default" "production"]; - platforms = []; - source = { - remotes = ["https://rubygems.org"]; - sha256 = "0ig48f4xhrssq5d11vkc41k7nj6pbv2jh1f8k5gfskfd469mcc2y"; - type = "gem"; - }; - version = "5.8.0"; - }; - unicorn-worker-killer = { - dependencies = ["get_process_mem" "unicorn"]; - groups = ["production"]; - platforms = []; - source = { - remotes = ["https://rubygems.org"]; - sha256 = "0rrdxpwdsapx47axjin8ymxb4f685qlpx8a26bql4ay1559c3gva"; - type = "gem"; - }; - version = "0.4.4"; - }; validate_email = { dependencies = ["activemodel" "mail"]; groups = ["default" "opf_plugins"]; diff --git a/packaging/scripts/check b/packaging/scripts/check index 25addd348cf..7d3fa8fd403 100755 --- a/packaging/scripts/check +++ b/packaging/scripts/check @@ -23,7 +23,7 @@ else log_ko "Web server is NOT running" fi -if ps -u "$APP_USER" -f | grep -q "unicorn worker" ; then +if ps -u "$APP_USER" -f | grep -qP "puma: cluster worker \d" ; then log_ok "openproject server is running" else log_ko "openproject server is NOT running" diff --git a/packaging/scripts/web b/packaging/scripts/web index 1a1db9f7ae5..2fc6c01c6ac 100755 --- a/packaging/scripts/web +++ b/packaging/scripts/web @@ -2,13 +2,5 @@ HOST="${HOST:=127.0.0.1}" PORT="${PORT:=8080}" -RAILS_ENV="${RAILS_ENV:="development"}" - -USE_PUMA="${USE_PUMA:="false"}" - -if [ "$USE_PUMA" = "true" ]; then - bundle exec rails server -u puma -b $HOST -p $PORT -else - bundle exec unicorn --config-file config/unicorn.rb --host $HOST --port $PORT --env $RAILS_ENV -fi +bundle exec rails server -u puma -b $HOST -p $PORT \ No newline at end of file