DEV: Add Pitchfork alongside Unicorn (#35370)

This PR adds Pitchfork, as we want to move away from Unicorn ultimately.

Unicorn still boots by default, so there should be no disruption for
anyone.

To use Pitchfork instead of Unicorn, the `RUN_PITCHFORK` environment
variable must be set.
This will make `bin/rails s` and `config/unicorn_launcher` boot
Pitchfork. `unicorn_launcher` was patched because that way we can easily
switch between Unicorn and Pitchfork without having to change too many
things on the infra side.

The upgrader from the `docker_manager` plugin doesn’t work yet with
Pitchfork. This will be addressed in a future PR.

.licensed.yml

@@ -73,6 +73,7 @@ reviewed:
     - nio4r # MIT + BSD
     - omniauth # MIT
     - pg # Ruby
+    - pitchfork # Ruby or GPLv2/GPLv3
     - r2 # Apache-2.0 (Twitter)
     - raindrops # LGPL-2.1+
     - rubyzip # Ruby

Gemfile

@@ -195,6 +195,7 @@ gem "rack-mini-profiler", require: ["enable_rails_patches"]
 
 gem "unicorn", require: false, platform: :ruby
 gem "puma", require: false
+gem "pitchfork", require: false
 
 gem "rbtrace", require: false, platform: :mri
 

Gemfile.lock

@@ -404,6 +404,9 @@ GEM
     pg (1.6.2-x86_64-darwin)
     pg (1.6.2-x86_64-linux)
     pg (1.6.2-x86_64-linux-musl)
+    pitchfork (0.18.1)
+      logger
+      rack (>= 2.0)
     playwright-ruby-client (1.55.0)
       concurrent-ruby (>= 1.1.6)
       mime-types (>= 3.0)
@@ -831,6 +834,7 @@ DEPENDENCIES
   parallel_tests
   pdf-reader
   pg
+  pitchfork
   propshaft
   pry-rails
   pry-stack_explorer
@@ -1092,6 +1096,7 @@ CHECKSUMS
   pg (1.6.2-x86_64-darwin) sha256=c441a55723584e2ae41749bf26024d7ffdfe1841b442308ed50cd6b7fda04115
   pg (1.6.2-x86_64-linux) sha256=525f438137f2d1411a1ebcc4208ec35cb526b5a3b285a629355c73208506a8ea
   pg (1.6.2-x86_64-linux-musl) sha256=e5c8668ffeaf7a9c3458a3dcb002dffa6d8ee1fca9ae534ffef861d2b15644ca
+  pitchfork (0.18.1) sha256=98f294024352c208d28732f7c009eed0d9ccae48d878a3b42722687338713059
   playwright-ruby-client (1.55.0) sha256=60510791dbfbdda6d3ed6ce27ad1353961f77016c04cacbb2ade1d4da9633b0a
   pp (0.6.3) sha256=2951d514450b93ccfeb1df7d021cae0da16e0a7f95ee1e2273719669d0ab9df6
   prettier_print (1.2.1) sha256=a72838b5f23facff21f90a5423cdcdda19e4271092b41f4ea7f50b83929e6ff9

bin/docker/pitchfork

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+exec "$(dirname "$0")/exec" bin/pitchfork "$@"

bin/pitchfork

@@ -0,0 +1,84 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require "pathname"
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", Pathname.new(__FILE__).realpath)
+
+STDERR.puts <<~MESSAGE if defined?(Bundler)
+    WARNING: Using `bundle exec` to start the server is unnecessary, and will make startup slower. Use `bin/pitchfork`.
+  MESSAGE
+
+require "rubygems"
+require "bundler/setup"
+require "fileutils"
+
+dev_mode = false
+
+# in development do some fussing around, to automate config
+if !ARGV.include?("-E") && !ARGV.include?("--env") &&
+     (%w[development test].include?(ENV["RAILS_ENV"]) || !ENV["RAILS_ENV"])
+  dev_mode = true
+
+  if !ARGV.include?("-c") && !ARGV.include?("--config-file")
+    ARGV.push("-c")
+    ARGV.push(File.expand_path("../../config/pitchfork.conf.rb", Pathname.new(__FILE__).realpath))
+  end
+
+  # we do not want to listen on 2 ports, so lets fix it
+  if (idx = ARGV.index("-p")) && (port = ARGV[idx + 1].to_i) > 0
+    ENV["UNICORN_PORT"] ||= port.to_s
+  end
+
+  ENV["UNICORN_PORT"] ||= "9292"
+
+  if ARGV.delete("-x")
+    puts "Running without sidekiq"
+    ENV["UNICORN_SIDEKIQS"] = "0"
+  end
+
+  ENV["UNICORN_SIDEKIQS"] ||= "1"
+end
+
+if ARGV.include?("--help")
+  fork { load Gem.bin_path("pitchfork", "pitchfork") }
+  Process.wait
+  puts "Extra Discourse Options:"
+  puts "  -x                       run without sidekiq"
+  exit
+end
+
+# this dev_mode hackery enables the following to be used to restart pitchfork:
+#
+# pkill -USR2 -f 'ruby bin/pitchfork'
+#
+# This is handy if you want to bind a key to restarting pitchfork in dev
+
+if dev_mode
+  UNICORN_DEV_SUPERVISOR_PID = Process.pid
+
+  restart = true
+  while restart
+    restart = false
+    pid = fork { load Gem.bin_path("pitchfork", "pitchfork") }
+    done = false
+
+    Signal.trap("INT") do
+      # wait for parent to be done
+    end
+
+    Signal.trap("USR2") do
+      Process.kill("QUIT", pid)
+      puts "RESTARTING PITCHFORK"
+      restart = true
+    end
+
+    Signal.trap("TERM") { Process.kill("TERM", pid) }
+
+    while !done
+      sleep 1
+      done = Process.waitpid(pid, Process::WNOHANG)
+    end
+  end
+else
+  load Gem.bin_path("pitchfork", "pitchfork")
+end

bin/rails

@@ -10,9 +10,9 @@ if !ENV["RAILS_ENV"] && (ARGV[0] == "s" || ARGV[0] == "server") && Process.respo
 
   ENV["RAILS_LOGS_STDOUT"] ||= "1"
 
-  exec File.expand_path("unicorn", __dir__)
+  exec File.expand_path(ENV["RUN_PITCHFORK"] ? "pitchfork" : "unicorn", __dir__)
 end
 
-APP_PATH = File.expand_path('../config/application', __dir__)
-require_relative '../config/boot'
-require 'rails/commands'
+APP_PATH = File.expand_path("../config/application", __dir__)
+require_relative "../config/boot"
+require "rails/commands"

bin/unicorn

@@ -1,56 +1,28 @@
 #!/usr/bin/env ruby
 # frozen_string_literal: true
 
-require 'pathname'
-ENV['BUNDLE_GEMFILE'] ||= File.expand_path("../../Gemfile",
-  Pathname.new(__FILE__).realpath)
-RAILS_ROOT = File.expand_path("../../", Pathname.new(__FILE__).realpath)
+require "pathname"
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", Pathname.new(__FILE__).realpath)
 
-if defined? Bundler
-  STDERR.puts <<~MESSAGE
+STDERR.puts <<~MESSAGE if defined?(Bundler)
     WARNING: Using `bundle exec` to start the server is unnecessary, and will make startup slower. Use `bin/rails s` or `bin/unicorn`.
   MESSAGE
-end
 
-require 'rubygems'
-require 'bundler/setup'
-require 'digest'
-require 'fileutils'
+require "rubygems"
+require "bundler/setup"
+require "fileutils"
 
 dev_mode = false
 
-def ensure_cache_clean!
-  _all_plugin_directories = Pathname.new(RAILS_ROOT + '/plugins').children.select(&:directory?)
-  core_git_sha = `git rev-parse HEAD`.strip
-  plugins_combined_git_sha = `git ls-files -s plugins | git hash-object --stdin`.strip
-  client_locale_paths_digest =
-    Digest::SHA1.hexdigest(Dir.glob("#{RAILS_ROOT}/plugins/*/config/locales/client.*.yml").join)
-  super_sha =
-    Digest::SHA1.hexdigest(core_git_sha + plugins_combined_git_sha + client_locale_paths_digest)
-  hash_file = "#{RAILS_ROOT}/tmp/plugin-hash"
-
-  old_hash = File.exist?(hash_file) ? File.read(hash_file) : nil
-
-  if old_hash && old_hash != super_sha
-    FileUtils.rm_rf("#{RAILS_ROOT}/tmp/cache")
-  end
-
-  FileUtils.mkdir_p(RAILS_ROOT + "/tmp")
-  File.write(hash_file, super_sha)
-end
-
 # in development do some fussing around, to automate config
-if !ARGV.include?("-E") &&
-    !ARGV.include?("--env") &&
-    (["development", "test"].include?(ENV["RAILS_ENV"]) || !ENV["RAILS_ENV"])
-
+if !ARGV.include?("-E") && !ARGV.include?("--env") &&
+     (%w[development test].include?(ENV["RAILS_ENV"]) || !ENV["RAILS_ENV"])
   dev_mode = true
 
   ARGV.push("-N")
   if !ARGV.include?("-c") && !ARGV.include?("--config-file")
     ARGV.push("-c")
-    ARGV.push(File.expand_path("../../config/unicorn.conf.rb",
-      Pathname.new(__FILE__).realpath))
+    ARGV.push(File.expand_path("../../config/unicorn.conf.rb", Pathname.new(__FILE__).realpath))
   end
 
   # we do not want to listen on 2 ports, so lets fix it
@@ -66,14 +38,10 @@ if !ARGV.include?("-E") &&
   end
 
   ENV["UNICORN_SIDEKIQS"] ||= "1"
-
-  ensure_cache_clean!
 end
 
 if ARGV.include?("--help")
-  fork do
-    load Gem.bin_path('unicorn', 'unicorn')
-  end
+  fork { load Gem.bin_path("unicorn", "unicorn") }
   Process.wait
   puts "Extra Discourse Options:"
   puts "  -x                       run without sidekiq"
@@ -92,24 +60,20 @@ if dev_mode
   restart = true
   while restart
     restart = false
-    pid = fork do
-      load Gem.bin_path('unicorn', 'unicorn')
-    end
+    pid = fork { load Gem.bin_path("unicorn", "unicorn") }
     done = false
 
-    Signal.trap('INT') do
+    Signal.trap("INT") do
       # wait for parent to be done
     end
 
-    Signal.trap('USR2') do
-      Process.kill('QUIT', pid)
+    Signal.trap("USR2") do
+      Process.kill("QUIT", pid)
       puts "RESTARTING UNICORN"
       restart = true
     end
 
-    Signal.trap("TERM") do
-      Process.kill('TERM', pid)
-    end
+    Signal.trap("TERM") { Process.kill("TERM", pid) }
 
     while !done
       sleep 1
@@ -117,5 +81,5 @@ if dev_mode
     end
   end
 else
-  load Gem.bin_path('unicorn', 'unicorn')
+  load Gem.bin_path("unicorn", "unicorn")
 end

config/environments/development.rb

@@ -51,7 +51,7 @@ Discourse::Application.configure do
   if defined?(BetterErrors)
     BetterErrors::Middleware.allow_ip! ENV["TRUSTED_IP"] if ENV["TRUSTED_IP"]
 
-    if defined?(Unicorn) && ENV["UNICORN_WORKERS"].to_i != 1
+    if (defined?(Unicorn) || defined?(Pitchfork)) && ENV["UNICORN_WORKERS"].to_i != 1
       # BetterErrors doesn't work with multiple unicorn workers. Disable it to avoid confusion
       Rails.configuration.middleware.delete BetterErrors::Middleware
     end
@@ -75,7 +75,7 @@ Discourse::Application.configure do
   end
 
   if ENV["DISCOURSE_SKIP_CSS_WATCHER"] != "1" &&
-       (defined?(Rails::Server) || defined?(Puma) || defined?(Unicorn))
+       (defined?(Rails::Server) || defined?(Puma) || defined?(Unicorn) || defined?(Pitchfork))
     require "stylesheet/watcher"
     STDERR.puts "Starting CSS change watcher"
     @watcher = Stylesheet::Watcher.watch

config/initializers/000-zeitwerk.rb

@@ -54,4 +54,5 @@ Rails.autoloaders.main.ignore(
   "lib/freedom_patches",
   "lib/i18n/backend",
   "lib/unicorn_logstash_patch.rb",
+  "lib/pitchfork_logstash_patch.rb",
 )

config/pitchfork.conf.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+discourse_path = File.expand_path(File.expand_path(File.dirname(__FILE__)) + "/../")
+enable_logstash_logger = ENV["ENABLE_LOGSTASH_LOGGER"] == "1"
+unicorn_stderr_path = "#{discourse_path}/log/unicorn.stderr.log"
+
+if enable_logstash_logger
+  require_relative "../lib/discourse_logstash_logger"
+  require_relative "../lib/pitchfork_logstash_patch"
+  FileUtils.touch(unicorn_stderr_path) if !File.exist?(unicorn_stderr_path)
+  logger DiscourseLogstashLogger.logger(
+           logdev: unicorn_stderr_path,
+           type: :unicorn,
+           customize_event: lambda { |event| event["@timestamp"] = ::Time.now.utc },
+         )
+else
+  logger Logger.new(STDOUT)
+end
+
+worker_processes (ENV["UNICORN_WORKERS"] || 3).to_i
+
+# stree-ignore
+listen ENV["UNICORN_LISTENER"] || "#{(ENV["UNICORN_BIND_ALL"] ? "" : "127.0.0.1:")}#{(ENV["UNICORN_PORT"] || 3000).to_i}"
+
+if ENV["RAILS_ENV"] == "production"
+  # nuke workers after 30 seconds instead of 60 seconds (the default)
+  timeout 30
+else
+  # we want a longer timeout in dev cause first request can be really slow
+  timeout(ENV["UNICORN_TIMEOUT"] && ENV["UNICORN_TIMEOUT"].to_i || 60)
+end
+
+check_client_connection false
+
+before_fork { |server| Discourse.redis.close }
+
+after_mold_fork do |server, mold|
+  if mold.generation.zero?
+    Discourse.preload_rails!
+
+    supervisor = ENV["UNICORN_SUPERVISOR_PID"].to_i
+
+    if supervisor > 0
+      Thread.new do
+        while true
+          unless File.exist?("/proc/#{supervisor}")
+            server.logger.error "Kill self, supervisor is gone"
+            Process.kill "TERM", Process.pid
+          end
+          sleep 2
+        end
+      end
+    end
+  end
+
+  Discourse.redis.close
+  Discourse.before_fork
+end
+
+after_worker_fork do |server, worker|
+  DiscourseEvent.trigger(:web_fork_started)
+  Discourse.after_fork
+  SignalTrapLogger.instance.after_fork
+end
+
+before_service_worker_ready do |server, service_worker|
+  sidekiqs = ENV["UNICORN_SIDEKIQS"].to_i
+
+  if sidekiqs > 0
+    server.logger.info "starting #{sidekiqs} supervised sidekiqs"
+
+    require "demon/sidekiq"
+    Demon::Sidekiq.after_fork { DiscourseEvent.trigger(:sidekiq_fork_started) }
+    Demon::Sidekiq.start(sidekiqs, logger: server.logger)
+
+    if Discourse.enable_sidekiq_logging?
+      # Trap USR1, so we can re-issue to sidekiq workers
+      # but chain the default unicorn implementation as well
+      old_handler =
+        Signal.trap("USR1") do
+          old_handler.call
+
+          # We have seen Sidekiq processes getting stuck in production sporadically when log rotation happens.
+          # The cause is currently unknown but we suspect that it is related to the Unicorn master process and
+          # Sidekiq demon processes reopening logs at the same time as we noticed that Unicorn worker processes only
+          # reopen logs after the Unicorn master process is done. To workaround the problem, we are adding an arbitrary
+          # delay of 1 second to Sidekiq's log reopeing procedure. The 1 second delay should be
+          # more than enough for the Unicorn master process to finish reopening logs.
+          Demon::Sidekiq.kill("USR2")
+        end
+    end
+  end
+
+  enable_email_sync_demon = ENV["DISCOURSE_ENABLE_EMAIL_SYNC_DEMON"] == "true"
+
+  if enable_email_sync_demon
+    server.logger.info "starting up EmailSync demon"
+    Demon::EmailSync.start(1, logger: server.logger)
+  end
+
+  DiscoursePluginRegistry.demon_processes.each do |demon_class|
+    server.logger.info "starting #{demon_class.prefix} demon"
+    demon_class.start(1, logger: server.logger)
+  end
+
+  Thread.new do
+    while true
+      begin
+        sleep 60
+
+        if sidekiqs > 0
+          Demon::Sidekiq.ensure_running
+          Demon::Sidekiq.heartbeat_check
+          Demon::Sidekiq.rss_memory_check
+        end
+
+        if enable_email_sync_demon
+          Demon::EmailSync.ensure_running
+          Demon::EmailSync.check_email_sync_heartbeat
+        end
+
+        DiscoursePluginRegistry.demon_processes.each { |demon_class| demon_class.ensure_running }
+      rescue => e
+        Rails.logger.warn(
+          "Error in demon processes heartbeat check: #{e}\n#{e.backtrace.join("\n")}",
+        )
+      end
+    end
+  end
+end

config/unicorn_launcher

@@ -60,10 +60,18 @@ function on_reopenlogs()
 export UNICORN_SUPERVISOR_PID=$$
 
 trap on_exit EXIT
-trap on_reload USR2 HUP
-trap on_reopenlogs USR1
+if [[ -z "$RUN_PITCHFORK" ]]; then
+  trap on_reload USR2 HUP
+  trap on_reopenlogs USR1
+  unicorn $@ &
+else
+  args=()
+  for arg in "$@"; do
+    args+=("${arg/unicorn.conf.rb/pitchfork.conf.rb}")
+  done
+  pitchfork "${args[@]}" &
+fi
 
-unicorn $@ &
 UNICORN_PID=$!
 
 echo "supervisor pid: $UNICORN_SUPERVISOR_PID unicorn pid: $UNICORN_PID"

config/unicorn_upstart.conf

@@ -1,35 +0,0 @@
-# you can copy this file to /etc/init/discourse.conf and then start discourse with
-# initctl start discourse
-
-# It assumes Discourse is installed at /var/www/discourse
-# It assumes Discourse is running under the discourse user
-# It assumes an rvm based setup
-
-description "Unicorn upstart for discourse"
-
-stop on runlevel [06]
-
-setuid discourse
-setgid discourse
-
-respawn
-respawn limit 3 30
-
-
-script
-
-exec /bin/bash <<'EOT'
-
-  # set HOME to the setuid user's home, there doesn't seem to be a better, portable way
-  export HOME="$(eval echo ~$(id -un))"
-  export RAILS_ENV=production
-
-  cd /var/www/discourse
-
-  source "$HOME/.rvm/scripts/rvm"
-
-   exec bundle exec unicorn -c config/unicorn.conf.rb
-EOT
-
-end script
-

lib/demon/sidekiq.rb

@@ -109,12 +109,14 @@ class Demon::Sidekiq < ::Demon::Base
     require "sidekiq/cli"
     cli = Sidekiq::CLI.instance
 
-    # Unicorn uses USR1 to indicate that log files have been rotated
-    Signal.trap("USR1") { reopen_logs }
+    if defined?(Unicorn)
+      # Unicorn uses USR1 to indicate that log files have been rotated
+      Signal.trap("USR1") { reopen_logs }
 
-    Signal.trap("USR2") do
-      sleep 1
-      reopen_logs
+      Signal.trap("USR2") do
+        sleep 1
+        reopen_logs
+      end
     end
 
     options = [

lib/discourse.rb

@@ -931,10 +931,7 @@ module Discourse
     ObjectSpace.each_object(MiniRacer::Context) { |c| c.dispose }
 
     # get rid of rubbish so we don't share it
-    # longer term we will use compact! here
-    GC.start
-    GC.start
-    GC.start
+    Process.warmup
   end
 
   # all forking servers must call this

lib/pitchfork_logstash_patch.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+# See https://github.com/Shopify/pitchfork/blob/18869d2f02549a54d7b2db6e0351e7fa71e95546/lib/pitchfork.rb#L120
+# Pitchfork originally logs backtrace line by line with `exc.backtrace.each { |line| logger.error(line) }`.
+# However, that means we get a separate logstash message for each backtrace which isn't what we want. The
+# monkey patch here overrides Pitchfork's logging of error so that we log the error and backtrace in a
+# single message.
+module Pitchfork
+  def self.log_error(logger, prefix, exc)
+    message = exc.message
+    message = message.dump if /[[:cntrl:]]/ =~ message
+    logger.error "#{prefix}: #{message} (#{exc.class})\n#{exc.backtrace.join("\n")}"
+  end
+end