From 8c0ccaec331c0c8f9cea0ff11cf16ba6244281b6 Mon Sep 17 00:00:00 2001 From: Aleksei Lipniagov Date: Mon, 24 May 2021 17:21:20 +0300 Subject: [PATCH] Remove Unicorn references from runtime logic Remove Unicorn references from Runtime and updated related specs. Changelog: removed --- .rubocop_manual_todo.yml | 1 - config/initializers/rbtrace.rb | 4 +- config/unicorn.rb.example | 144 ------------------------- config/unicorn.rb.example.development | 77 ------------- lib/gitlab/cluster/lifecycle_events.rb | 3 - lib/gitlab/runtime.rb | 10 +- spec/rack_servers/unicorn_spec.rb | 105 ------------------ 7 files changed, 4 insertions(+), 340 deletions(-) delete mode 100644 config/unicorn.rb.example delete mode 100644 config/unicorn.rb.example.development delete mode 100644 spec/rack_servers/unicorn_spec.rb diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 4fb55ecaf6bb09..ae15995215a841 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -1309,7 +1309,6 @@ RSpec/AnyInstanceOf: - 'spec/support/shared_examples/workers/authorized_projects_worker_shared_example.rb' - 'spec/support/shared_examples/workers/reactive_cacheable_shared_examples.rb' - 'spec/support/snowplow.rb' - - 'spec/support/unicorn.rb' - 'spec/tasks/gitlab/cleanup_rake_spec.rb' - 'spec/tasks/gitlab/container_registry_rake_spec.rb' - 'spec/tasks/gitlab/db_rake_spec.rb' diff --git a/config/initializers/rbtrace.rb b/config/initializers/rbtrace.rb index 6a1b71bf4bdb80..2359fc9f6b5aaf 100644 --- a/config/initializers/rbtrace.rb +++ b/config/initializers/rbtrace.rb @@ -2,8 +2,8 @@ if ENV['ENABLE_RBTRACE'] Gitlab::Cluster::LifecycleEvents.on_worker_start do - # Unicorn clears out signals before it forks, so rbtrace won't work - # unless it is enabled after the fork. + # We need to require `rbtrace` in a context of a worker process. + # See https://github.com/tmm1/rbtrace/issues/56#issuecomment-648683596. require 'rbtrace' end end diff --git a/config/unicorn.rb.example b/config/unicorn.rb.example deleted file mode 100644 index c930e2ff761f43..00000000000000 --- a/config/unicorn.rb.example +++ /dev/null @@ -1,144 +0,0 @@ -# Sample verbose configuration file for Unicorn (not Rack) -# -# This configuration file documents many features of Unicorn -# that may not be needed for some applications. See -# http://unicorn.bogomips.org/examples/unicorn.conf.minimal.rb -# for a much simpler configuration file. -# -# See http://unicorn.bogomips.org/Unicorn/Configurator.html for complete -# documentation. - -# Note: If you change this file in a merge request, please also create a -# merge request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests - -# Relative URL support -# WARNING: We recommend using an FQDN to host GitLab in a root path instead -# of using a relative URL. -# Documentation: http://doc.gitlab.com/ce/install/relative_url.html -# Uncomment and customize the following line to run in a non-root path -# -# ENV['RAILS_RELATIVE_URL_ROOT'] = "/gitlab" - -# Read about unicorn workers here: -# http://doc.gitlab.com/ee/install/requirements.html#unicorn-workers -# -worker_processes 3 - -# Since Unicorn is never exposed to outside clients, it does not need to -# run on the standard HTTP port (80), there is no reason to start Unicorn -# as root unless it's from system init scripts. -# If running the master process as root and the workers as an unprivileged -# user, do this to switch euid/egid in the workers (also chowns logs): -# user "unprivileged_user", "unprivileged_group" - -# Help ensure your application will always spawn in the symlinked -# "current" directory that Capistrano sets up. -working_directory "/home/git/gitlab" # available in 0.94.0+ - -# Listen on both a Unix domain socket and a TCP port. -# If you are load-balancing multiple Unicorn masters, lower the backlog -# setting to e.g. 64 for faster failover. -listen "/home/git/gitlab/tmp/sockets/gitlab.socket", :backlog => 1024 -listen "127.0.0.1:8080", :tcp_nopush => true - -# destroy workers after 30 seconds instead of 60 seconds (the default) -# -# NOTICE: git push over http depends on this value. -# If you want to be able to push huge amount of data to git repository over http -# you will have to increase this value too. -# -# Example of output if you try to push 1GB repo to GitLab over http. -# -> git push http://gitlab.... master -# -# error: RPC failed; result=18, HTTP code = 200 -# fatal: The remote end hung up unexpectedly -# fatal: The remote end hung up unexpectedly -# -# For more information see http://stackoverflow.com/a/21682112/752049 -# -timeout 60 - -# feel free to point this anywhere accessible on the filesystem -pid "/home/git/gitlab/tmp/pids/unicorn.pid" - -# By default, the Unicorn logger will write to stderr. -# Additionally, some applications/frameworks log to stderr or stdout, -# so prevent them from going to /dev/null when daemonized here: -stderr_path "/home/git/gitlab/log/unicorn.stderr.log" -stdout_path "/home/git/gitlab/log/unicorn.stdout.log" - -# Save memory by sharing the application code among multiple Unicorn workers -# with "preload_app true". See: -# https://www.rubydoc.info/gems/unicorn/5.1.0/Unicorn%2FConfigurator:preload_app -# https://brandur.org/ruby-memory#copy-on-write -preload_app true - -# Enable this flag to have unicorn test client connections by writing the -# beginning of the HTTP headers before calling the application. This -# prevents calling the application for connections that have disconnected -# while queued. This is only guaranteed to detect clients on the same -# host unicorn runs on, and unlikely to detect disconnects even on a -# fast LAN. -check_client_connection false - -require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events" -require_relative "/home/git/gitlab/lib/gitlab/log_timestamp_formatter.rb" - -before_exec do |server| - # Signal application hooks that we're about to restart - Gitlab::Cluster::LifecycleEvents.do_before_master_restart -end - -run_once = true - -before_fork do |server, worker| - if run_once - # There is a difference between Puma and Unicorn: - # - Puma calls before_fork once when booting up master process - # - Unicorn runs before_fork whenever new work is spawned - # To unify this behavior we call before_fork only once (we use - # this callback for deleting Prometheus files so for our purposes - # it makes sense to align behavior with Puma) - run_once = false - - # Signal application hooks that we're about to fork - Gitlab::Cluster::LifecycleEvents.do_before_fork - end - - # The following is only recommended for memory/DB-constrained - # installations. It is not needed if your system can house - # twice as many worker_processes as you have configured. - # - # This allows a new master process to incrementally - # phase out the old master process with SIGTTOU to avoid a - # thundering herd (especially in the "preload_app false" case) - # when doing a transparent upgrade. The last worker spawned - # will then kill off the old master process with a SIGQUIT. - old_pid = "#{server.config[:pid]}.oldbin" - if old_pid != server.pid - begin - sig = (worker.nr + 1) >= server.worker_processes ? :QUIT : :TTOU - Process.kill(sig, File.read(old_pid).to_i) - rescue Errno::ENOENT, Errno::ESRCH - end - end - # - # Throttle the master from forking too quickly by sleeping. Due - # to the implementation of standard Unix signal handlers, this - # helps (but does not completely) prevent identical, repeated signals - # from being lost when the receiving process is busy. - # sleep 1 -end - -after_fork do |server, worker| - # Signal application hooks of worker start - Gitlab::Cluster::LifecycleEvents.do_worker_start - - # per-process listener ports for debugging/admin/migrations - # addr = "127.0.0.1:#{9293 + worker.nr}" - # server.listen(addr, :tries => -1, :delay => 5, :tcp_nopush => true) -end - -# Configure the default logger to use a custom formatter that formats the -# timestamps to be in UTC and in ISO8601.3 format -Configurator::DEFAULTS[:logger].formatter = Gitlab::LogTimestampFormatter.new diff --git a/config/unicorn.rb.example.development b/config/unicorn.rb.example.development deleted file mode 100644 index 2c6e809f753b94..00000000000000 --- a/config/unicorn.rb.example.development +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true - -# ------------------------------------------------------------------------- -# This file is used by the GDK to generate a default config/unicorn.rb file -# Note that `/home/git` will be substituted for the actual GDK root -# directory when this file is generated -# ------------------------------------------------------------------------- - -worker_processes 2 -timeout 60 - -listen '/home/git/gitlab.socket' - -preload_app true -check_client_connection false - -require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events" -require_relative "/home/git/gitlab/lib/gitlab/log_timestamp_formatter.rb" - -before_exec do |server| - # Signal application hooks that we're about to restart - Gitlab::Cluster::LifecycleEvents.do_before_master_restart -end - -run_once = true - -before_fork do |server, worker| - if run_once - # There is a difference between Puma and Unicorn: - # - Puma calls before_fork once when booting up master process - # - Unicorn runs before_fork whenever new work is spawned - # To unify this behavior we call before_fork only once (we use - # this callback for deleting Prometheus files so for our purposes - # it makes sense to align behavior with Puma) - run_once = false - - # Signal application hooks that we're about to fork - Gitlab::Cluster::LifecycleEvents.do_before_fork - end - - # The following is only recommended for memory/DB-constrained - # installations. It is not needed if your system can house - # twice as many worker_processes as you have configured. - # - # This allows a new master process to incrementally - # phase out the old master process with SIGTTOU to avoid a - # thundering herd (especially in the "preload_app false" case) - # when doing a transparent upgrade. The last worker spawned - # will then kill off the old master process with a SIGQUIT. - old_pid = "#{server.config[:pid]}.oldbin" - if old_pid != server.pid - begin - sig = (worker.nr + 1) >= server.worker_processes ? :QUIT : :TTOU - Process.kill(sig, File.read(old_pid).to_i) - rescue Errno::ENOENT, Errno::ESRCH - end - end - # - # Throttle the master from forking too quickly by sleeping. Due - # to the implementation of standard Unix signal handlers, this - # helps (but does not completely) prevent identical, repeated signals - # from being lost when the receiving process is busy. - # sleep 1 -end - -after_fork do |server, worker| - # Signal application hooks of worker start - Gitlab::Cluster::LifecycleEvents.do_worker_start - - # per-process listener ports for debugging/admin/migrations - # addr = "127.0.0.1:#{9293 + worker.nr}" - # server.listen(addr, :tries => -1, :delay => 5, :tcp_nopush => true) -end - -# Configure the default logger to use a custom formatter that formats the -# timestamps to be in UTC and in ISO8601.3 format -Configurator::DEFAULTS[:logger].formatter = Gitlab::LogTimestampFormatter.new diff --git a/lib/gitlab/cluster/lifecycle_events.rb b/lib/gitlab/cluster/lifecycle_events.rb index 6b7b9b09269008..6159fb0a811213 100644 --- a/lib/gitlab/cluster/lifecycle_events.rb +++ b/lib/gitlab/cluster/lifecycle_events.rb @@ -162,9 +162,6 @@ def in_clustered_environment? # Sidekiq doesn't fork return false if Gitlab::Runtime.sidekiq? - # Unicorn always forks - return true if Gitlab::Runtime.unicorn? - # Puma sometimes forks return true if in_clustered_puma? diff --git a/lib/gitlab/runtime.rb b/lib/gitlab/runtime.rb index b0bcea0ca69054..f60cac0aff0cce 100644 --- a/lib/gitlab/runtime.rb +++ b/lib/gitlab/runtime.rb @@ -15,8 +15,7 @@ module Runtime :rails_runner, :rake, :sidekiq, - :test_suite, - :unicorn + :test_suite ].freeze class << self @@ -36,11 +35,6 @@ def puma? !!defined?(::Puma) end - # For unicorn, we need to check for actual server instances to avoid false positives. - def unicorn? - !!(defined?(::Unicorn) && defined?(::Unicorn::HttpServer)) - end - def sidekiq? !!(defined?(::Sidekiq) && Sidekiq.server?) end @@ -66,7 +60,7 @@ def rails_runner? end def web_server? - puma? || unicorn? + puma? end def action_cable? diff --git a/spec/rack_servers/unicorn_spec.rb b/spec/rack_servers/unicorn_spec.rb deleted file mode 100644 index 52d44b6e7e0a35..00000000000000 --- a/spec/rack_servers/unicorn_spec.rb +++ /dev/null @@ -1,105 +0,0 @@ -# frozen_string_literal: true - -require 'fileutils' - -require 'excon' - -require 'spec_helper' - -RSpec.describe 'Unicorn' do - before(:all) do - project_root = File.expand_path('../..', __dir__) - - config_lines = File.read('config/unicorn.rb.example') - .gsub('/home/git/gitlab', project_root) - .gsub('/home/git', project_root) - .split("\n") - - # Remove these because they make setup harder. - config_lines = config_lines.reject do |line| - %w[ - worker_processes - listen - pid - stderr_path - stdout_path - ].any? { |prefix| line.start_with?(prefix) } - end - - config_lines << "working_directory '#{Rails.root}'" - - # We want to have exactly 1 worker process because that makes it - # predictable which process will handle our requests. - config_lines << 'worker_processes 1' - - @socket_path = File.join(project_root, 'tmp/tests/unicorn.socket') - config_lines << "listen '#{@socket_path}'" - - ready_file = File.join(project_root, 'tmp/tests/unicorn-worker-ready') - FileUtils.rm_f(ready_file) - after_fork_index = config_lines.index { |l| l.start_with?('after_fork') } - config_lines.insert(after_fork_index + 1, "File.write('#{ready_file}', Process.pid)") - - config_path = File.join(project_root, 'tmp/tests/unicorn.rb') - File.write(config_path, config_lines.join("\n") + "\n") - - cmd = %W[unicorn -E test -c #{config_path} spec/rack_servers/configs/config.ru] - @unicorn_master_pid = spawn(*cmd) - wait_unicorn_boot!(@unicorn_master_pid, ready_file) - WebMock.allow_net_connect! - end - - %w[SIGQUIT SIGTERM SIGKILL].each do |signal| - it "has a worker that self-terminates on signal #{signal}" do - response = Excon.get('unix://', socket: @socket_path) - expect(response.status).to eq(200) - - worker_pid = response.body.to_i - expect(worker_pid).to be > 0 - - begin - Excon.post("unix://?#{signal}", socket: @socket_path) - rescue Excon::Error::Socket - # The connection may be closed abruptly - end - - expect(pid_gone?(worker_pid)).to eq(true) - end - end - - after(:all) do - webmock_enable! - Process.kill('TERM', @unicorn_master_pid) - end - - def wait_unicorn_boot!(master_pid, ready_file) - # We have seen the boot timeout after 2 minutes in CI so let's set it to 5 minutes. - timeout = 5 * 60 - timeout.times do - return if File.exist?(ready_file) - - pid = Process.waitpid(master_pid, Process::WNOHANG) - raise "unicorn failed to boot: #{$?}" unless pid.nil? - - sleep 1 - end - - raise "unicorn boot timed out after #{timeout} seconds" - end - - def pid_gone?(pid) - # Worker termination should take less than a second. That makes 10 - # seconds a generous timeout. - 10.times do - begin - Process.kill(0, pid) - rescue Errno::ESRCH - return true - end - - sleep 1 - end - - false - end -end -- GitLab