Add experimental support for Puma

This allows us (and others) to test drive Puma without it affecting all
users. Puma can be enabled by setting the environment variable
"EXPERIMENTAL_PUMA" to a non empty value.

.gitignore

@@ -40,6 +40,7 @@ eslint-report.html
 /config/redis.queues.yml
 /config/redis.shared_state.yml
 /config/unicorn.rb
+/config/puma.rb
 /config/secrets.yml
 /config/sidekiq.yml
 /config/registry.key

Gemfile

@@ -157,6 +157,11 @@ group :unicorn do
   gem 'unicorn-worker-killer', '~> 0.4.4'
 end
+group :puma do
+  gem 'puma', '~> 3.12', require: false
+  gem 'puma_worker_killer', require: false
+end
+
 # State machine
 gem 'state_machines-activerecord', '~> 0.5.1'

Gemfile.lock

@@ -624,6 +624,10 @@ GEM
     pry-rails (0.3.6)
       pry (>= 0.10.4)
     public_suffix (3.0.3)
+    puma (3.12.0)
+    puma_worker_killer (0.1.0)
+      get_process_mem (~> 0.2)
+      puma (>= 2.7, < 4)
     pyu-ruby-sasl (0.0.3.3)
     rack (1.6.10)
     rack-accept (0.4.5)
@@ -1104,6 +1108,8 @@ DEPENDENCIES
   prometheus-client-mmap (~> 0.9.4)
   pry-byebug (~> 3.4.1)
   pry-rails (~> 0.3.4)
+  puma (~> 3.12)
+  puma_worker_killer
   rack-attack (~> 4.4.1)
   rack-cors (~> 1.0.0)
   rack-oauth2 (~> 1.2.1)

Gemfile.rails5.lock

@@ -628,6 +628,10 @@ GEM
     pry-rails (0.3.6)
       pry (>= 0.10.4)
     public_suffix (3.0.3)
+    puma (3.12.0)
+    puma_worker_killer (0.1.0)
+      get_process_mem (~> 0.2)
+      puma (>= 2.7, < 4)
     pyu-ruby-sasl (0.0.3.3)
     rack (2.0.5)
     rack-accept (0.4.5)
@@ -1113,6 +1117,8 @@ DEPENDENCIES
   prometheus-client-mmap (~> 0.9.4)
   pry-byebug (~> 3.4.1)
   pry-rails (~> 0.3.4)
+  puma (~> 3.12)
+  puma_worker_killer
   rack-attack (~> 4.4.1)
   rack-cors (~> 1.0.0)
   rack-oauth2 (~> 1.2.1)

bin/web

@@ -3,6 +3,12 @@
 cd $(dirname $0)/..
 app_root=$(pwd)
+
+# Switch to experimental PUMA configuration
+if [ -n "${EXPERIMENTAL_PUMA}" ]; then
+  exec bin/web_puma "$@"
+fi
+
 unicorn_pidfile="$app_root/tmp/pids/unicorn.pid"
 unicorn_config="$app_root/config/unicorn.rb"
 unicorn_cmd="bundle exec unicorn_rails -c $unicorn_config -E $RAILS_ENV"

bin/web_puma

+#!/bin/sh
+
+set -e
+
+cd $(dirname $0)/..
+app_root=$(pwd)
+
+puma_pidfile="$app_root/tmp/pids/puma.pid"
+puma_config="$app_root/config/puma.rb"
+
+spawn_puma()
+{
+  exec bundle exec puma --config "${puma_config}" "$@"
+}
+
+get_puma_pid()
+{
+  pid=$(cat "${puma_pidfile}")
+  if [ -z "$pid" ] ; then
+    echo "Could not find a PID in $puma_pidfile"
+    exit 1
+  fi
+  echo "${pid}"
+}
+
+start()
+{
+  spawn_puma -d
+}
+
+start_foreground()
+{
+  spawn_puma
+}
+
+stop()
+{
+  get_puma_pid
+  kill -QUIT "$(get_puma_pid)"
+}
+
+reload()
+{
+  kill -USR2 "$(get_puma_pid)"
+}
+
+case "$1" in
+  start)
+    start
+    ;;
+  start_foreground)
+    start_foreground
+    ;;
+  stop)
+    stop
+    ;;
+  reload)
+    reload
+    ;;
+  *)
+    echo "Usage: RAILS_ENV=your_env $0 {start|stop|reload}"
+    ;;
+esac

changelogs/unreleased/an-multithreading.yml

+---
+title: Puma in GDK
+merge_request: 22372
+author:
+type: performance

config/initializers/7_prometheus_metrics.rb

@@ -26,9 +26,24 @@ Sidekiq.configure_server do |config|
 end
 if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
-  unless Sidekiq.server?
-    Gitlab::Metrics::Samplers::UnicornSampler.initialize_instance(Settings.monitoring.unicorn_sampler_interval).start
+  Gitlab::Cluster::LifecycleEvents.on_worker_start do
+    defined?(::Prometheus::Client.reinitialize_on_pid_change) && Prometheus::Client.reinitialize_on_pid_change
+
+    unless Sidekiq.server?
+      Gitlab::Metrics::Samplers::UnicornSampler.initialize_instance(Settings.monitoring.unicorn_sampler_interval).start
+    end
+
+    Gitlab::Metrics::Samplers::RubySampler.initialize_instance(Settings.monitoring.ruby_sampler_interval).start
   end
-  Gitlab::Metrics::Samplers::RubySampler.initialize_instance(Settings.monitoring.ruby_sampler_interval).start
+end
+Gitlab::Cluster::LifecycleEvents.signal_master_restart do
+  # The following is necessary to ensure stale Prometheus metrics don't
+  # accumulate over time. It needs to be done in this hook as opposed to
+  # inside an init script to ensure metrics files aren't deleted after new
+  # unicorn workers start after a SIGUSR2 is received.
+  if ENV['prometheus_multiproc_dir']
+    old_metrics = Dir[File.join(ENV['prometheus_multiproc_dir'], '*.db')]
+    FileUtils.rm_rf(old_metrics)
+  end
 end

config/initializers/8_metrics.rb

@@ -158,7 +158,9 @@ if Gitlab::Metrics.enabled? && !Rails.env.test?
   GC::Profiler.enable
-  Gitlab::Metrics::Samplers::InfluxSampler.initialize_instance.start
+  Gitlab::Cluster::LifecycleEvents.on_worker_start do
+    Gitlab::Metrics::Samplers::InfluxSampler.initialize_instance.start
+  end
   module TrackNewRedisConnections
     def connect(*args)

config/initializers/active_record_lifecycle.rb

+# frozen_string_literal: true
+
+# Don't handle sidekiq configuration as it
+# has it's own special active record configuration here
+if defined?(ActiveRecord::Base) && !Sidekiq.server?
+  Gitlab::Cluster::LifecycleEvents.on_worker_start do
+    ActiveSupport.on_load(:active_record) do
+      ActiveRecord::Base.establish_connection
+
+      Rails.logger.debug("ActiveRecord connection established")
+    end
+  end
+end
+
+if defined?(ActiveRecord::Base)
+  Gitlab::Cluster::LifecycleEvents.on_before_fork do
+    # the following is highly recommended for Rails + "preload_app true"
+    # as there's no need for the master process to hold a connection
+    ActiveRecord::Base.connection.disconnect!
+
+    Rails.logger.debug("ActiveRecord connection disconnected")
+  end
+end

config/initializers/macos.rb

+# frozen_string_literal: true
+
+if /darwin/ =~ RUBY_PLATFORM
+  Gitlab::Cluster::LifecycleEvents.on_before_fork do
+    require 'fiddle'
+
+    # Dynamically load Foundation.framework, ~implicitly~ initialising
+    # the Objective-C runtime before any forking happens in Unicorn
+    #
+    # From https://bugs.ruby-lang.org/issues/14009
+    Fiddle.dlopen '/System/Library/Frameworks/Foundation.framework/Foundation'
+  end
+end

config/initializers/rbtrace.rb

+# frozen_string_literal: true
+
+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.
+    require 'rbtrace'
+  end
+end

config/initializers/sidekiq.rb

@@ -14,8 +14,6 @@ Sidekiq.default_worker_options = { retry: 3 }
 enable_json_logs = Gitlab.config.sidekiq.log_format == 'json'
 Sidekiq.configure_server do |config|
-  require 'rbtrace' if ENV['ENABLE_RBTRACE']
-
   config.redis = queues_config_hash
   config.server_middleware do |chain|

config/puma.example.development.rb

+# frozen_string_literal: true
+
+before_fork do |server, worker|
+  if /darwin/ =~ RUBY_PLATFORM
+    require 'fiddle'
+
+    # Dynamically load Foundation.framework, ~implicitly~ initialising
+    # the Objective-C runtime before any forking happens in Unicorn
+    #
+    # From https://bugs.ruby-lang.org/issues/14009
+    Fiddle.dlopen '/System/Library/Frameworks/Foundation.framework/Foundation'
+  end
+end
+
+# Load "path" as a rackup file.
+#
+# The default is "config.ru".
+#
+rackup 'config.ru'
+pidfile '$GDK_ROOT/gitlab/tmp/pids/puma.pid'
+state_path '$GDK_ROOT/gitlab/tmp/pids/puma.state'
+
+stdout_redirect '$GDK_ROOT/gitlab/log/puma.stdout.log',
+  '$GDK_ROOT/gitlab/log/puma.stderr.log',
+  true
+
+# Configure "min" to be the minimum number of threads to use to answer
+# requests and "max" the maximum.
+#
+# The default is "0, 16".
+#
+threads 1, 1
+
+# By default, workers accept all requests and queue them to pass to handlers.
+# When false, workers accept the number of simultaneous requests configured.
+#
+# Queueing requests generally improves performance, but can cause deadlocks if
+# the app is waiting on a request to itself. See https://github.com/puma/puma/issues/612
+#
+# When set to false this may require a reverse proxy to handle slow clients and
+# queue requests before they reach puma. This is due to disabling HTTP keepalive
+queue_requests false
+
+# Bind the server to "url". "tcp://", "unix://" and "ssl://" are the only
+# accepted protocols.
+bind 'unix://$GDK_ROOT/gitlab.socket'
+
+on_restart do
+  ActiveRecord::Base.connection.disconnect! if defined?(ActiveRecord::Base)
+end
+
+workers 2
+
+def require_from_app(path)
+  # We cannot control where this file is, so can't use require_relative directly,
+  # but we know that working_directory and `Dir.pwd` will always be the root of
+  # the application
+  require 'pathname'
+  required_module_path = Pathname.new(Dir.pwd()) + path
+  require_relative required_module_path.to_s
+end
+
+require_from_app "./lib/gitlab/cluster/lifecycle_events"
+require_from_app "./lib/gitlab/cluster/puma_worker_killer_initializer"
+
+before_fork do
+  Gitlab::Cluster::PumaWorkerKillerInitializer.start(@config)
+  Gitlab::Cluster::LifecycleEvents.signal_before_fork
+end
+
+Gitlab::Cluster::LifecycleEvents.set_puma_options @config.options
+on_worker_boot do
+  Gitlab::Cluster::LifecycleEvents.signal_worker_start
+end
+
+on_restart do
+  Gitlab::Cluster::LifecycleEvents.signal_master_restart
+end
+
+# Preload the application before starting the workers; this conflicts with
+# phased restart feature. (off by default)
+
+preload_app!
+
+tag 'gitlab-puma-worker'
+
+# Verifies that all workers have checked in to the master process within
+# the given timeout. If not the worker process will be restarted. Default
+# value is 60 seconds.
+#
+worker_timeout 60

config/unicorn.rb.example

@@ -81,22 +81,19 @@ preload_app true
 # fast LAN.
 check_client_connection false
+# We cannot control where unicorn.rb is but
+# we know that working_directory and `Dir.pwd` will always be the
+# root of the application
+require 'pathname'
+required_module_path = Pathname.new(Dir.pwd()) + "lib/gitlab/cluster/lifecycle_events"
+require_relative required_module_path.to_s
+
 before_exec do |server|
-  # The following is necessary to ensure stale Prometheus metrics don't
-  # accumulate over time. It needs to be done in this hook as opposed to
-  # inside an init script to ensure metrics files aren't deleted after new
-  # unicorn workers start after a SIGUSR2 is received.
-  if ENV['prometheus_multiproc_dir']
-    old_metrics = Dir[File.join(ENV['prometheus_multiproc_dir'], '*.db')]
-    FileUtils.rm_rf(old_metrics)
-  end
+  Gitlab::Cluster::LifecycleEvents.signal_master_restart
 end
 before_fork do |server, worker|
-  # the following is highly recommended for Rails + "preload_app true"
-  # as there's no need for the master process to hold a connection
-  defined?(ActiveRecord::Base) &&
-    ActiveRecord::Base.connection.disconnect!
+  Gitlab::Cluster::LifecycleEvents.signal_before_fork
   # The following is only recommended for memory/DB-constrained
   # installations.  It is not needed if your system can house
@@ -124,25 +121,9 @@ before_fork do |server, worker|
 end
 after_fork do |server, worker|
-  # Unicorn clears out signals before it forks, so rbtrace won't work
-  # unless it is enabled after the fork.
-  require 'rbtrace' if ENV['ENABLE_RBTRACE']
+  Gitlab::Cluster::LifecycleEvents.signal_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)
-
-  # the following is *required* for Rails + "preload_app true",
-  defined?(ActiveRecord::Base) &&
-    ActiveRecord::Base.establish_connection
-
-  # reset prometheus client, this will cause any opened metrics files to be closed
-  defined?(::Prometheus::Client.reinitialize_on_pid_change) &&
-    Prometheus::Client.reinitialize_on_pid_change
-
-  # if preload_app is true, then you may also want to check and
-  # restart any other shared sockets/descriptors such as Memcached,
-  # and Redis.  TokyoCabinet file handles are safe to reuse
-  # between any number of forked children (assuming your kernel
-  # correctly implements pread()/pwrite() system calls)
 end

config/unicorn.rb.example.development

@@ -4,29 +4,50 @@ timeout 60
 preload_app true
 check_client_connection false
+# We cannot control where unicorn.rb is but
+# we know that working_directory and `Dir.pwd` will always be the
+# root of the application
+require 'pathname'
+required_module_path = Pathname.new(Dir.pwd()) + "lib/gitlab/cluster/lifecycle_events"
+require_relative required_module_path.to_s
+
+before_exec do |server|
+  Gitlab::Cluster::LifecycleEvents.signal_master_restart
+end
+
 before_fork do |server, worker|
-  # the following is highly recommended for Rails + "preload_app true"
-  # as there's no need for the master process to hold a connection
-  defined?(ActiveRecord::Base) &&
-    ActiveRecord::Base.connection.disconnect!
-
-  if /darwin/ =~ RUBY_PLATFORM
-    require 'fiddle'
-
-    # Dynamically load Foundation.framework, ~implicitly~ initialising
-    # the Objective-C runtime before any forking happens in Unicorn
-    #
-    # From https://bugs.ruby-lang.org/issues/14009
-    Fiddle.dlopen '/System/Library/Frameworks/Foundation.framework/Foundation'
+  Gitlab::Cluster::LifecycleEvents.signal_before_fork
+
+  # 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|
-  # Unicorn clears out signals before it forks, so rbtrace won't work
-  # unless it is enabled after the fork.
-  require 'rbtrace' if ENV['ENABLE_RBTRACE']
-  # the following is *required* for Rails + "preload_app true",
-  defined?(ActiveRecord::Base) &&
-    ActiveRecord::Base.establish_connection
+  Gitlab::Cluster::LifecycleEvents.signal_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
+

lib/gitlab/cluster/lifecycle_events.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Cluster
+    # This class abstracts various lifecycle events for different runtime environments
+    # This allows handlers for various events to be registered and executed regardless
+    # of the environment. Possible environments considered while building this class
+    # include Unicorn, Puma Single, Puma Clustered, Sidekiq Multithreaded Process, Ruby,
+    # Rake, rails-console etc
+    #
+    # Blocks will be executed in the order in which they are registered.
+    class LifecycleEvents
+      # Initialization lifecycle event. Any block registered can expect to be
+      # executed once per process. In the event of single process environments,
+      # the block is executed immediately
+      def self.on_worker_start(&block)
+        if in_clustered_worker?
+          (@worker_start_listeners ||= []) << block
+        else
+          block.call
+        end
+      end
+
+      # Lifecycle event in the master process to signal that a child is about to be
+      # forked
+      def self.on_before_fork(&block)
+        (@before_fork_listeners ||= []) << block
+      end
+
+      # Lifecycle event for main process restart. Signals that the main process should
+      # restart.
+      def self.on_master_restart(&block)
+        (@master_restart_listeners ||= []) << block
+      end
+
+      # Signal worker_start event
+      # This should be called from unicorn/puma/etc lifecycle hooks
+      def self.signal_worker_start
+        @worker_start_listeners && @worker_start_listeners.each do |block|
+          block.call
+        end
+      end
+
+      # Signal before_fork event
+      # This should be called from unicorn/puma/etc lifecycle hooks
+      def self.signal_before_fork
+        @before_fork_listeners && @before_fork_listeners.each do |block|
+          block.call
+        end
+      end
+
+      # Signal master_restart event
+      # This should be called from unicorn/puma/etc lifecycle hooks
+      def self.signal_master_restart
+        @master_restart_listeners && @master_restart_listeners.each do |block|
+          block.call
+        end
+      end
+
+      # Returns true for environments which fork worker processes,
+      # noteably Puma in cluster mode and unicorn
+      def self.in_clustered_worker?
+        # Sidekiq doesn't fork
+        return false if Sidekiq.server?
+
+        # Unicorn always forks
+        return true if defined?(::Unicorn)
+
+        # Puma sometimes forks
+        return true if in_clustered_puma?
+
+        # Default assumption is that we don't fork
+        false
+      end
+      private_class_method :in_clustered_worker?
+
+      # Returns true when running in Puma in clustered mode
+      def self.in_clustered_puma?
+        return false unless defined?(::Puma)
+
+        @puma_options && @puma_options[:workers] && @puma_options[:workers] > 0
+      end
+      private_class_method :in_clustered_puma?
+
+      # Puma doesn't use singletons (which is good) but
+      # this means we need to pass through whether the
+      # puma server is running in single mode or cluster mode
+      def self.set_puma_options(options)
+        @puma_options = options
+      end
+    end
+  end
+end

lib/gitlab/cluster/puma_worker_killer_initializer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Cluster
+    class PumaWorkerKillerInitializer
+      def self.start(puma_config)
+        require 'puma_worker_killer'
+
+        PumaWorkerKiller.config do |config|
+          # For now we use the same environment variable as Unicorn, making it
+          # easier to replace Unicorn with Puma.
+
+          # Note! ram is expressed in megabytes, whereas GITLAB_UNICORN_MEMORY_MAX is in bytes
+          # Importantly RAM is for _all_workers, not each worker as is the case with GITLAB_UNICORN_MEMORY_MAX
+          worker_count = puma_config.options[:workers] || 1
+          config.ram = worker_count * (ENV['GITLAB_UNICORN_MEMORY_MAX'] ? ENV['GITLAB_UNICORN_MEMORY_MAX'].to_i / (1 << 20) : 650)
+
+          config.frequency = 20 # seconds
+
+          # We just wan't to limit to a fixed maximum, unrelated to the total amount
+          # of available RAM.
+          config.percent_usage = 0.98
+
+          # Ideally we'll never hit the maximum amount of memory. If so the worker
+          # is restarted already, thus periodically restarting workers shouldn't be
+          # needed.
+          config.rolling_restart_frequency = false
+        end
+
+        PumaWorkerKiller.start
+      end
+    end
+  end
+end

spec/rack_servers/configs/config.ru

+# frozen_string_literal: true
+
+app = proc do |env|
+  if env['REQUEST_METHOD'] == 'GET'
+    [200, {}, ["#{Process.pid}"]]
+  else
+    Process.kill(env['QUERY_STRING'], Process.pid)
+    [200, {}, ['Bye!']]
+  end
+end
+
+run app

spec/rack_servers/configs/puma.rb

+# frozen_string_literal: true
+
+# The directory to operate out of.
+#
+# The default is the current directory.
+#
+directory '$WORKING_DIR'
+
+# Configure "min" to be the minimum number of threads to use to answer
+# requests and "max" the maximum.
+#
+# The default is "0, 16".
+threads 1, 10
+
+# By default, workers accept all requests and queue them to pass to handlers.
+# When false, workers accept the number of simultaneous requests configured.
+#
+# Queueing requests generally improves performance, but can cause deadlocks if
+# the app is waiting on a request to itself. See https://github.com/puma/puma/issues/612
+#
+# When set to false this may require a reverse proxy to handle slow clients and
+# queue requests before they reach puma. This is due to disabling HTTP keepalive
+queue_requests false
+
+# Bind the server to "url". "tcp://", "unix://" and "ssl://" are the only
+# accepted protocols.
+bind 'unix://$WORKING_DIR/tmp/tests/puma.socket'
+
+workers 1
+
+def require_from_app(path)
+  # We cannot control where this file is, so can't use require_relative directly,
+  # but we know that working_directory and `Dir.pwd` will always be the root of
+  # the application
+  require 'pathname'
+  required_module_path = Pathname.new(Dir.pwd()) + path
+  require_relative required_module_path.to_s
+end
+
+require_from_app "./lib/gitlab/cluster/lifecycle_events"
+require_from_app "./lib/gitlab/cluster/puma_worker_killer_initializer"
+
+before_fork do
+  Gitlab::Cluster::PumaWorkerKillerInitializer.start(@config)
+  Gitlab::Cluster::LifecycleEvents.signal_before_fork
+end
+
+Gitlab::Cluster::LifecycleEvents.set_puma_options @config.options
+on_worker_boot do
+  Gitlab::Cluster::LifecycleEvents.signal_worker_start
+  File.write('tmp/tests/puma-worker-ready', Process.pid)
+end
+
+on_restart do
+  Gitlab::Cluster::LifecycleEvents.signal_master_restart
+end
+
+# Preload the application before starting the workers; this conflicts with
+# phased restart feature. (off by default)
+
+preload_app!
+
+tag 'gitlab-puma-worker'
+
+# Verifies that all workers have checked in to the master process within
+# the given timeout. If not the worker process will be restarted. Default
+# value is 60 secon ds.
+#
+worker_timeout 60