From 4fe49fe5591c5ba21de8065983f46d0b1b4023d1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 5 Jun 2021 11:13:25 +0100 Subject: [PATCH 1/4] Switch from Unicorn to Puma --- Gemfile | 4 ++-- Gemfile.lock | 20 +++++--------------- config.ru | 13 ------------- 3 files changed, 7 insertions(+), 30 deletions(-) diff --git a/Gemfile b/Gemfile index 126b63a797..f73f8a7780 100644 --- a/Gemfile +++ b/Gemfile @@ -83,6 +83,7 @@ gem 'rack-rewrite' gem 'rack-ssl', require: 'rack/ssl' gem 'roadie-rails' +gem 'puma' gem 'hiredis' gem 'redis', '>= 4.0', require: ['redis', 'redis/connection/hiredis'] gem 'sidekiq' @@ -128,7 +129,7 @@ gem "view_component", require: "view_component/engine" group :production, :staging do gem 'ddtrace' - gem 'unicorn-worker-killer' + gem 'sd_notify' # For better Systemd process management. Used by Puma. end group :test, :development do @@ -148,7 +149,6 @@ group :test, :development do gem 'selenium-webdriver' gem 'shoulda-matchers' gem 'timecop' - gem 'unicorn-rails' gem 'webdrivers' gem 'cuprite' end diff --git a/Gemfile.lock b/Gemfile.lock index 784ad40705..5c5fd23259 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -319,8 +319,6 @@ GEM rspec-core (~> 3.0) ruby-progressbar (~> 1.4) geocoder (1.6.7) - get_process_mem (0.2.7) - ffi (~> 1.0) globalid (0.4.2) activesupport (>= 4.2.0) gmaps4rails (2.1.2) @@ -355,7 +353,6 @@ GEM multi_json (~> 1.0) rspec (>= 2.0, < 4.0) jwt (2.2.3) - kgio (2.11.3) knapsack (3.1.0) rake launchy (2.5.0) @@ -435,6 +432,8 @@ GEM byebug (~> 11.0) pry (~> 0.13.0) public_suffix (4.0.6) + puma (5.3.2) + nio4r (~> 2.0) raabro (1.4.0) racc (1.5.2) rack (2.2.3) @@ -484,7 +483,6 @@ GEM rake (>= 0.13) thor (~> 1.0) rainbow (3.0.0) - raindrops (0.19.1) rake (13.0.3) ransack (2.4.2) activerecord (>= 5.2.4) @@ -575,6 +573,7 @@ GEM sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) tilt (>= 1.1, < 3) + sd_notify (0.1.1) selenium-webdriver (3.142.7) childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) @@ -632,15 +631,6 @@ GEM uglifier (4.2.0) execjs (>= 0.3.0, < 3) unicode-display_width (2.0.0) - unicorn (6.0.0) - kgio (~> 2.6) - raindrops (~> 0.7) - unicorn-rails (2.2.1) - rack - unicorn - unicorn-worker-killer (0.4.5) - get_process_mem (~> 0) - unicorn (>= 4, < 7) uniform_notifier (1.14.2) valid_email2 (4.0.0) activemodel (>= 3.2) @@ -769,6 +759,7 @@ DEPENDENCIES pg (~> 1.2.3) pry (~> 0.13.0) pry-byebug (~> 3.9.0) + puma rack-mini-profiler (< 3.0.0) rack-rewrite rack-ssl @@ -788,6 +779,7 @@ DEPENDENCIES rubocop rubocop-rails sass-rails (< 5.1.0) + sd_notify select2-rails! selenium-webdriver shoulda-matchers @@ -803,8 +795,6 @@ DEPENDENCIES test-unit (~> 3.4) timecop uglifier (>= 1.0.3) - unicorn-rails - unicorn-worker-killer valid_email2 view_component view_component_storybook diff --git a/config.ru b/config.ru index 0e1070856d..b834259514 100644 --- a/config.ru +++ b/config.ru @@ -2,18 +2,5 @@ # This file is used by Rack-based servers to start the application. -if ENV.fetch('KILL_UNICORNS', false) && ['production', 'staging'].include?(ENV['RAILS_ENV']) - # Gracefully restart individual unicorn workers if they have: - # - performed between 25000 and 30000 requests - # - grown in memory usage to between 700 and 850 MB - require 'unicorn/worker_killer' - use Unicorn::WorkerKiller::MaxRequests, - ENV.fetch('UWK_REQS_MIN', 25_000).to_i, - ENV.fetch('UWK_REQS_MAX', 30_000).to_i - use Unicorn::WorkerKiller::Oom, - ( ENV.fetch('UWK_MEM_MIN', 700).to_i * (1024**2) ), - ( ENV.fetch('UWK_MEM_MAX', 850).to_i * (1024**2) ) -end - require ::File.expand_path('config/environment', __dir__) run Openfoodnetwork::Application From bd041a2e09afb8c3ad3619b2abacf8c5dffbf924 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 26 Jul 2021 18:47:17 +0100 Subject: [PATCH 2/4] Use puma in the test suite and remove server errors hack --- spec/base_spec_helper.rb | 2 +- spec/features/consumer/shopping/shopping_spec.rb | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 121fac8f27..58b6623a86 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -36,7 +36,7 @@ WebMock.disable_net_connect!( # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f } -Capybara.server = :webrick +Capybara.server = :puma Capybara.configure do |config| config.match = :prefer_exact diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb index 00a00ac777..b027b48eef 100644 --- a/spec/features/consumer/shopping/shopping_spec.rb +++ b/spec/features/consumer/shopping/shopping_spec.rb @@ -167,12 +167,6 @@ feature "As a consumer I want to shop with a distributor", js: true do end describe "two order cycles and more than 20 products for each" do - around do |example| - Capybara.raise_server_errors = false - example.run - Capybara.raise_server_errors = true - end - before do 20.times do product = create(:simple_product, supplier: supplier) From 1b112961d17eb67ed3e4c80a03d6b1343f093eda Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 31 Jul 2021 12:46:13 +0100 Subject: [PATCH 3/4] Handle request timeouts explicitly with rack-timeout Puma doesn't terminate execution of long-running requests by default. --- Gemfile | 1 + Gemfile.lock | 2 ++ app/controllers/api/v0/base_controller.rb | 1 + app/controllers/application_controller.rb | 2 ++ app/controllers/concerns/request_timeouts.rb | 23 ++++++++++++++++++++ 5 files changed, 29 insertions(+) create mode 100644 app/controllers/concerns/request_timeouts.rb diff --git a/Gemfile b/Gemfile index f73f8a7780..61ec5ea1fb 100644 --- a/Gemfile +++ b/Gemfile @@ -129,6 +129,7 @@ gem "view_component", require: "view_component/engine" group :production, :staging do gem 'ddtrace' + gem 'rack-timeout' gem 'sd_notify' # For better Systemd process management. Used by Puma. end diff --git a/Gemfile.lock b/Gemfile.lock index 5c5fd23259..043ff0750b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -448,6 +448,7 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) + rack-timeout (0.6.0) rails (6.1.4) actioncable (= 6.1.4) actionmailbox (= 6.1.4) @@ -763,6 +764,7 @@ DEPENDENCIES rack-mini-profiler (< 3.0.0) rack-rewrite rack-ssl + rack-timeout rails (~> 6.1.4) rails-controller-testing rails-i18n diff --git a/app/controllers/api/v0/base_controller.rb b/app/controllers/api/v0/base_controller.rb index 1f89cb6c68..a911455cb0 100644 --- a/app/controllers/api/v0/base_controller.rb +++ b/app/controllers/api/v0/base_controller.rb @@ -14,6 +14,7 @@ module Api include ::ActionController::Head include ::ActionController::ConditionalGet include ActionView::Layouts + include RequestTimeouts layout false diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 757cecd578..9971d6dedb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -10,6 +10,8 @@ require 'open_food_network/referer_parser' class ApplicationController < ActionController::Base include Pagy::Backend + include RequestTimeouts + self.responder = ApplicationResponder respond_to :html diff --git a/app/controllers/concerns/request_timeouts.rb b/app/controllers/concerns/request_timeouts.rb new file mode 100644 index 0000000000..6653872aa7 --- /dev/null +++ b/app/controllers/concerns/request_timeouts.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RequestTimeouts + extend ActiveSupport::Concern + + included do + rescue_from Rack::Timeout::RequestTimeoutException, with: :timeout_response if defined? Rack::Timeout + end + + private + + def timeout_response(_exception = nil) + respond_to do |type| + type.html { + render status: :gateway_timeout, + file: Rails.root.join("public/500.html"), + formats: [:html], + layout: nil + } + type.all { render status: :gateway_timeout, body: nil } + end + end +end From 5725989343653bb52a0fe7563992d92b374a8f29 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 31 Jul 2021 12:47:06 +0100 Subject: [PATCH 4/4] Fix deprecated syntax in old controller helper This syntax for rendering a file isn't valid in Rails 6.x... --- lib/spree/core/controller_helpers/common.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spree/core/controller_helpers/common.rb b/lib/spree/core/controller_helpers/common.rb index 78625c0bd8..27684873b4 100644 --- a/lib/spree/core/controller_helpers/common.rb +++ b/lib/spree/core/controller_helpers/common.rb @@ -42,7 +42,7 @@ module Spree respond_to do |type| type.html { render status: :not_found, - file: "#{::Rails.root}/public/404", + file: Rails.root.join("public/404.html"), formats: [:html], layout: nil }