diff --git a/config/unicorn.conf.rb b/config/unicorn.conf.rb index 5260334dca7..fd2bf2ae0e9 100644 --- a/config/unicorn.conf.rb +++ b/config/unicorn.conf.rb @@ -134,7 +134,7 @@ before_fork do |server, worker| if enable_email_sync_demon Demon::EmailSync.ensure_running - check_email_sync_heartbeat + Demon::EmailSync.check_email_sync_heartbeat end DiscoursePluginRegistry.demon_processes.each { |demon_class| demon_class.ensure_running } @@ -180,60 +180,6 @@ before_fork do |server, worker| end next_sleep <= 0 ? 1 : next_sleep end - - def max_email_sync_rss - return 0 if Demon::EmailSync.demons.empty? - - email_sync_pids = Demon::EmailSync.demons.map { |uid, demon| demon.pid } - return 0 if email_sync_pids.empty? - - rss = - `ps -eo pid,rss,args | grep '#{email_sync_pids.join("|")}' | grep -v grep | awk '{print $2}'`.split( - "\n", - ) - .map(&:to_i) - .max - - (rss || 0) * 1024 - end - - def max_allowed_email_sync_rss - [ENV["UNICORN_EMAIL_SYNC_MAX_RSS"].to_i, 500].max.megabytes - end - - def check_email_sync_heartbeat - # Skip first check to let process warm up - @email_sync_next_heartbeat_check ||= (Time.now + Demon::EmailSync::HEARTBEAT_INTERVAL).to_i - - return if @email_sync_next_heartbeat_check > Time.now.to_i - @email_sync_next_heartbeat_check = (Time.now + Demon::EmailSync::HEARTBEAT_INTERVAL).to_i - - restart = false - - # Restart process if it does not respond anymore - last_heartbeat_ago = - Time.now.to_i - Discourse.redis.get(Demon::EmailSync::HEARTBEAT_KEY).to_i - if last_heartbeat_ago > Demon::EmailSync::HEARTBEAT_INTERVAL.to_i - Rails.logger.warn( - "EmailSync heartbeat test failed (last heartbeat was #{last_heartbeat_ago}s ago), restarting", - ) - - restart = true - end - - # Restart process if memory usage is too high - email_sync_rss = max_email_sync_rss - if email_sync_rss > max_allowed_email_sync_rss - Rails.logger.warn( - "EmailSync is consuming too much memory (using: %0.2fM) for '%s', restarting" % - [(email_sync_rss.to_f / 1.megabyte), ENV["DISCOURSE_HOSTNAME"]], - ) - - restart = true - end - - Demon::EmailSync.restart if restart - end end end diff --git a/lib/demon/base.rb b/lib/demon/base.rb index 4ebc5ade908..98bdc905ac7 100644 --- a/lib/demon/base.rb +++ b/lib/demon/base.rb @@ -5,6 +5,8 @@ end # intelligent fork based demonizer class Demon::Base + HOSTNAME = Socket.gethostname + def self.demons @demons end diff --git a/lib/demon/email_sync.rb b/lib/demon/email_sync.rb index 1d6b3c56a09..9f25c4f06b0 100644 --- a/lib/demon/email_sync.rb +++ b/lib/demon/email_sync.rb @@ -10,6 +10,74 @@ class Demon::EmailSync < ::Demon::Base "email_sync" end + def self.max_email_sync_rss + return 0 if demons.empty? + + email_sync_pids = demons.map { |uid, demon| demon.pid } + + return 0 if email_sync_pids.empty? + + rss = + `ps -eo pid,rss,args | grep '#{email_sync_pids.join("|")}' | grep -v grep | awk '{print $2}'`.split( + "\n", + ) + .map(&:to_i) + .max + + (rss || 0) * 1024 + end + private_class_method :max_email_sync_rss + + DEFAULT_MAX_ALLOWED_EMAIL_SYNC_RSS_MEGABYTES = 500 + + def self.max_allowed_email_sync_rss + [ + ENV["UNICORN_EMAIL_SYNC_MAX_RSS"].to_i, + DEFAULT_MAX_ALLOWED_EMAIL_SYNC_RSS_MEGABYTES, + ].max.megabytes + end + private_class_method :max_allowed_email_sync_rss + + if Rails.env.test? + def self.test_cleanup + @@email_sync_next_heartbeat_check = nil + end + end + + def self.check_email_sync_heartbeat + if defined?(@@email_sync_next_heartbeat_check) && @@email_sync_next_heartbeat_check && + @@email_sync_next_heartbeat_check > Time.now.to_i + return + end + + @@email_sync_next_heartbeat_check = (Time.now + HEARTBEAT_INTERVAL).to_i + + should_restart = false + + # Restart process if it does not respond anymore + last_heartbeat_ago = Time.now.to_i - Discourse.redis.get(HEARTBEAT_KEY).to_i + + if last_heartbeat_ago > HEARTBEAT_INTERVAL.to_i + Rails.logger.warn( + "EmailSync heartbeat test failed (last heartbeat was #{last_heartbeat_ago}s ago), restarting", + ) + + should_restart = true + end + + # Restart process if memory usage is too high + if !should_restart && (email_sync_rss = max_email_sync_rss) > max_allowed_email_sync_rss + Rails.logger.warn( + "EmailSync is consuming too much memory (using: %0.2fM) for '%s', restarting" % + [(email_sync_rss.to_f / 1.megabyte), HOSTNAME], + ) + + should_restart = true + end + + restart if should_restart + end + private def suppress_stdout diff --git a/lib/demon/sidekiq.rb b/lib/demon/sidekiq.rb index df689e5db05..d90daac5aa1 100644 --- a/lib/demon/sidekiq.rb +++ b/lib/demon/sidekiq.rb @@ -3,8 +3,6 @@ require "demon/base" class Demon::Sidekiq < ::Demon::Base - HOSTNAME = Socket.gethostname - def self.prefix "sidekiq" end diff --git a/spec/lib/demon/email_sync_spec.rb b/spec/lib/demon/email_sync_spec.rb new file mode 100644 index 00000000000..0477d095595 --- /dev/null +++ b/spec/lib/demon/email_sync_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe Demon::EmailSync do + describe ".check_email_sync_heartbeat" do + after do + described_class.reset_demons + described_class.test_cleanup + end + + it "should restart email sync daemons when last heartbeat is older than the heartbeat interval" do + track_log_messages do |logger| + daemon = described_class.new(1) + daemon.set_pid(999_999) + + described_class.set_demons({ "daemon" => daemon }) + + Discourse.redis.set( + described_class::HEARTBEAT_KEY, + Time.now.to_i - described_class::HEARTBEAT_INTERVAL.to_i - 1, + ) + + daemon.expects(:restart) + + described_class.check_email_sync_heartbeat + + expect(logger.warnings.first).to eq( + "EmailSync heartbeat test failed (last heartbeat was 61s ago), restarting", + ) + end + end + + it "should restart email sync daemons when memory usage exceeds the maximum allowed memory" do + track_log_messages do |logger| + daemon = described_class.new(1) + daemon.set_pid(999_999) + + described_class.set_demons({ "daemon" => daemon }) + + Discourse.redis.set(described_class::HEARTBEAT_KEY, Time.now.to_i) + + daemon.expects(:restart) + + # Set to negative value to fake that process is using too much memory + described_class.expects(:max_allowed_email_sync_rss).returns(-1) + described_class.check_email_sync_heartbeat + + expect(logger.warnings.first).to eq( + "EmailSync is consuming too much memory (using: 0.00M) for '#{described_class::HOSTNAME}', restarting", + ) + end + end + end +end