From 878f06f1fe0fb7027f3bb81b00ec145c94bae17a Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 21 May 2020 14:32:41 +0800 Subject: [PATCH] DEV: Remove custom connection reaper. Rails 6 fixed the reaper to use one thread to reap all the connection pools. --- app/models/global_setting.rb | 3 ++- config/application.rb | 5 ---- config/initializers/099-drain_pool.rb | 4 --- lib/discourse.rb | 35 --------------------------- lib/freedom_patches/reaper.rb | 20 --------------- 5 files changed, 2 insertions(+), 65 deletions(-) delete mode 100644 lib/freedom_patches/reaper.rb diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index 21e5a137dcf..890f5ef06c6 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -144,8 +144,9 @@ class GlobalSetting hash["host_names"] = hostnames hash["database"] = db_name - hash["prepared_statements"] = !!self.db_prepared_statements + hash["idle_timeout"] = connection_reaper_age if connection_reaper_age.present? + hash["reaping_frequency"] = connection_reaper_interval if connection_reaper_interval.present? { "production" => hash } end diff --git a/config/application.rb b/config/application.rb index 0157dc82895..808f9e4592b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -360,10 +360,5 @@ module Discourse config.generators do |g| g.test_framework :rspec, fixture: false end - - # we have a monkey_patch we need to require early... prior to connection - # init - require 'freedom_patches/reaper' - end end diff --git a/config/initializers/099-drain_pool.rb b/config/initializers/099-drain_pool.rb index cff95339b8a..e69de29bb2d 100644 --- a/config/initializers/099-drain_pool.rb +++ b/config/initializers/099-drain_pool.rb @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -# pg performs inconsistently with large amounts of connections -Discourse.start_connection_reaper diff --git a/lib/discourse.rb b/lib/discourse.rb index 9bdbc2b3024..094ca70aec8 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -660,7 +660,6 @@ module Discourse Sidekiq.redis_pool.shutdown { |c| nil } # re-establish Sidekiq.redis = sidekiq_redis_config - start_connection_reaper # in case v8 was initialized we want to make sure it is nil PrettyText.reset_context @@ -743,40 +742,6 @@ module Discourse STDERR.puts "Failed to report exception #{e} #{message}" end - def self.start_connection_reaper - return if GlobalSetting.connection_reaper_age < 1 || - GlobalSetting.connection_reaper_interval < 1 - - # this helps keep connection counts in check - Thread.new do - while true - begin - sleep GlobalSetting.connection_reaper_interval - reap_connections(GlobalSetting.connection_reaper_age) - rescue => e - Discourse.warn_exception(e, message: "Error reaping connections") - end - end - end - end - - def self.reap_connections(idle) - pools = [] - ObjectSpace.each_object(ActiveRecord::ConnectionAdapters::ConnectionPool) { |pool| pools << pool } - - pools.each do |pool| - # reap recovers connections that were aborted - # eg a thread died or a dev forgot to check it in - # flush removes idle connections - # after fork we have "deadpools" so ignore them, they have been discarded - # so @connections is set to nil - if pool.connections - pool.reap - pool.flush(idle) - end - end - end - def self.deprecate(warning, drop_from: nil, since: nil, raise_error: false, output_in_test: false) location = caller_locations[1].yield_self { |l| "#{l.path}:#{l.lineno}:in \`#{l.label}\`" } warning = ["Deprecation notice:", warning] diff --git a/lib/freedom_patches/reaper.rb b/lib/freedom_patches/reaper.rb deleted file mode 100644 index dc49a528f84..00000000000 --- a/lib/freedom_patches/reaper.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -# Discourse ships with a connection reaper -# this patch ensures that the connection reaper never runs in Rails -# -# In Rails 5.2 the connection reaper is "per-pool" this means it can bloat -# threads quite a lot in a multisite -# -# Note, the "correct" way is to set this in the spec, however due to multisite -# getting reaper_interval=0 into all the specs is not going to be trivial -# when we eventually do that we can remove this patch - -if !defined? ActiveRecord::ConnectionAdapters::ConnectionPool::Reaper - raise "Can not find connection Reaper class, this patch will no longer work!" -end - -class ActiveRecord::ConnectionAdapters::ConnectionPool::Reaper - def run - end -end