From 58e52c0e4fd5fcd35627fa89db48a5719cfa3e45 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 11 Jun 2020 13:45:46 +0800 Subject: [PATCH] DEV: Use rails_failover gem for ActiveRecord and Redis failover handling --- Gemfile | 2 +- Gemfile.lock | 13 +- app/models/global_setting.rb | 30 +-- config/application.rb | 10 +- config/initializers/002-rails_failover.rb | 112 ++++------- config/initializers/200-first_middlewares.rb | 11 +- .../postgresql_fallback_adapter.rb | 190 ------------------ lib/discourse.rb | 32 ++- lib/discourse_redis.rb | 147 -------------- lib/freedom_patches/zeitwerk.rb | 1 - spec/models/global_setting_spec.rb | 2 +- 11 files changed, 84 insertions(+), 466 deletions(-) delete mode 100644 lib/active_record/connection_adapters/postgresql_fallback_adapter.rb diff --git a/Gemfile b/Gemfile index 8e4acf64cd7..4d809fda20a 100644 --- a/Gemfile +++ b/Gemfile @@ -250,4 +250,4 @@ gem 'webpush', require: false gem 'colored2', require: false gem 'maxminddb' -gem 'rails_failover', require: false, git: 'https://github.com/discourse/rails_failover' +gem 'rails_failover', require: false diff --git a/Gemfile.lock b/Gemfile.lock index b181c5e4aa4..d838afb0fc3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,11 +1,3 @@ -GIT - remote: https://github.com/discourse/rails_failover - revision: 66602aa73785851b81c506f0023d3c2a2e40de0a - specs: - rails_failover (0.4.0) - activerecord (~> 6.0) - railties (~> 6.0) - GEM remote: https://rubygems.org/ specs: @@ -288,6 +280,9 @@ GEM nokogiri (>= 1.6) rails-html-sanitizer (1.3.0) loofah (~> 2.3) + rails_failover (0.5.0) + activerecord (~> 6.0) + railties (~> 6.0) rails_multisite (2.3.0) activerecord (> 5.0, < 7) railties (> 5.0, < 7) @@ -526,7 +521,7 @@ DEPENDENCIES rack (= 2.2.2) rack-mini-profiler rack-protection - rails_failover! + rails_failover rails_multisite railties (= 6.0.3.1) rake diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index 1d8df226791..dc660fce50a 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -134,12 +134,6 @@ class GlobalSetting end end - if hash["replica_host"] - if !ENV["ACTIVE_RECORD_RAILS_FAILOVER"] - hash["adapter"] = "postgresql_fallback" - end - end - hostnames = [ hostname ] hostnames << backup_hostname if backup_hostname.present? @@ -170,15 +164,9 @@ class GlobalSetting c[:port] = redis_port if redis_port if redis_slave_host && redis_slave_port - if ENV["REDIS_RAILS_FAILOVER"] - c[:replica_host] = redis_slave_host - c[:replica_port] = redis_slave_port - c[:connector] = RailsFailover::Redis::Connector - else - c[:slave_host] = redis_slave_host - c[:slave_port] = redis_slave_port - c[:connector] = DiscourseRedis::Connector - end + c[:replica_host] = redis_slave_host + c[:replica_port] = redis_slave_port + c[:connector] = RailsFailover::Redis::Connector end c[:password] = redis_password if redis_password.present? @@ -200,15 +188,9 @@ class GlobalSetting c[:port] = message_bus_redis_port if message_bus_redis_port if message_bus_redis_slave_host && message_bus_redis_slave_port - if ENV["REDIS_RAILS_FAILOVER"] - c[:replica_host] = message_bus_redis_slave_host - c[:replica_port] = message_bus_redis_slave_port - c[:connector] = RailsFailover::Redis::Connector - else - c[:slave_host] = message_bus_redis_slave_host - c[:slave_port] = message_bus_redis_slave_port - c[:connector] = DiscourseRedis::Connector - end + c[:replica_host] = message_bus_redis_slave_host + c[:replica_port] = message_bus_redis_slave_port + c[:connector] = RailsFailover::Redis::Connector end c[:password] = message_bus_redis_password if message_bus_redis_password.present? diff --git a/config/application.rb b/config/application.rb index a93c86c258c..0e17596d8d9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -19,6 +19,8 @@ require 'action_controller/railtie' require 'action_view/railtie' require 'action_mailer/railtie' require 'sprockets/railtie' +require 'rails_failover/active_record' +require 'rails_failover/redis' # Plugin related stuff require_relative '../lib/plugin_initialization_guard' @@ -27,14 +29,6 @@ require_relative '../lib/discourse_plugin_registry' require_relative '../lib/plugin_gem' -if ENV['ACTIVE_RECORD_RAILS_FAILOVER'] - require 'rails_failover/active_record' -end - -if ENV['REDIS_RAILS_FAILOVER'] - require 'rails_failover/redis' -end - # Global config require_relative '../app/models/global_setting' GlobalSetting.configure! diff --git a/config/initializers/002-rails_failover.rb b/config/initializers/002-rails_failover.rb index 6952bc95f0c..1c0edc93e6e 100644 --- a/config/initializers/002-rails_failover.rb +++ b/config/initializers/002-rails_failover.rb @@ -1,81 +1,51 @@ # frozen_string_literal: true -if ENV["REDIS_RAILS_FAILOVER"] - message_bus_keepalive_interval = nil +message_bus_keepalive_interval = nil - RailsFailover::Redis.on_failover do - message_bus_keepalive_interval = MessageBus.keepalive_interval - MessageBus.keepalive_interval = -1 # Disable MessageBus keepalive_interval - Discourse.received_redis_readonly! - end +RailsFailover::Redis.on_failover do + message_bus_keepalive_interval = MessageBus.keepalive_interval + MessageBus.keepalive_interval = -1 # Disable MessageBus keepalive_interval + Discourse.received_redis_readonly! +end - RailsFailover::Redis.on_fallback do - Discourse.clear_redis_readonly! - Discourse.request_refresh! - MessageBus.keepalive_interval = message_bus_keepalive_interval +RailsFailover::Redis.on_fallback do + Discourse.clear_redis_readonly! + Discourse.request_refresh! + MessageBus.keepalive_interval = message_bus_keepalive_interval +end + +if Rails.configuration.multisite + if ActiveRecord::Base.current_role == ActiveRecord::Base.reading_role + RailsMultisite::ConnectionManagement.default_connection_handler = + ActiveRecord::Base.connection_handlers[ActiveRecord::Base.reading_role] end end -if ENV["ACTIVE_RECORD_RAILS_FAILOVER"] +RailsFailover::ActiveRecord.on_failover do + RailsMultisite::ConnectionManagement.each_connection do + Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) + Sidekiq.pause!("pg_failover") if !Sidekiq.paused? + end +end + +RailsFailover::ActiveRecord.on_fallback do + RailsMultisite::ConnectionManagement.each_connection do + Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) + Sidekiq.unpause! if Sidekiq.paused? + end + if Rails.configuration.multisite - if ActiveRecord::Base.current_role == ActiveRecord::Base.reading_role - RailsMultisite::ConnectionManagement.default_connection_handler = - ActiveRecord::Base.connection_handlers[ActiveRecord::Base.reading_role] - end - end - - RailsFailover::ActiveRecord.on_failover do - RailsMultisite::ConnectionManagement.each_connection do - Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) - Sidekiq.pause!("pg_failover") if !Sidekiq.paused? - end - rescue => e - Rails.logger.warn "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" - false - end - - RailsFailover::ActiveRecord.on_fallback do - RailsMultisite::ConnectionManagement.each_connection do - Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) - Sidekiq.unpause! if Sidekiq.paused? - end - - if Rails.configuration.multisite - RailsMultisite::ConnectionManagement.default_connection_handler = - ActiveRecord::Base.connection_handlers[ActiveRecord::Base.writing_role] - end - rescue => e - Rails.logger.warn "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" - false - end - - module Discourse - PG_FORCE_READONLY_MODE_KEY ||= 'readonly_mode:postgres_force' - - READONLY_KEYS.push(PG_FORCE_READONLY_MODE_KEY) - - def self.enable_pg_force_readonly_mode - Discourse.redis.set(PG_FORCE_READONLY_MODE_KEY, 1) - Sidekiq.pause!("pg_failover") if !Sidekiq.paused? - MessageBus.publish(readonly_channel, true) - true - end - - def self.disable_pg_force_readonly_mode - result = Discourse.redis.del(PG_FORCE_READONLY_MODE_KEY) - Sidekiq.unpause! - MessageBus.publish(readonly_channel, false) - result > 0 - end - end - - RailsFailover::ActiveRecord.register_force_reading_role_callback do - Discourse.redis.exists?( - Discourse::PG_READONLY_MODE_KEY, - Discourse::PG_FORCE_READONLY_MODE_KEY - ) - rescue => e - Rails.logger.warn "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" - false + RailsMultisite::ConnectionManagement.default_connection_handler = + ActiveRecord::Base.connection_handlers[ActiveRecord::Base.writing_role] end end + +RailsFailover::ActiveRecord.register_force_reading_role_callback do + Discourse.redis.exists?( + Discourse::PG_READONLY_MODE_KEY, + Discourse::PG_FORCE_READONLY_MODE_KEY + ) +rescue => e + Rails.logger.warn "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" + false +end diff --git a/config/initializers/200-first_middlewares.rb b/config/initializers/200-first_middlewares.rb index 5fada923cd0..57ef2aecbf3 100644 --- a/config/initializers/200-first_middlewares.rb +++ b/config/initializers/200-first_middlewares.rb @@ -24,12 +24,7 @@ if Rails.configuration.multisite # Multisite needs to be first, because the request tracker and message bus rely on it Rails.configuration.middleware.unshift RailsMultisite::Middleware, RailsMultisite::DiscoursePatches.config Rails.configuration.middleware.delete ActionDispatch::Executor -end - -if ENV["ACTIVE_RECORD_RAILS_FAILOVER"] - if Rails.configuration.multisite - Rails.configuration.middleware.insert_after(RailsMultisite::Middleware, RailsFailover::ActiveRecord::Middleware) - else - Rails.configuration.middleware.insert_before(MessageBus::Rack::Middleware, RailsFailover::ActiveRecord::Middleware) - end + Rails.configuration.middleware.insert_after(RailsMultisite::Middleware, RailsFailover::ActiveRecord::Middleware) +else + Rails.configuration.middleware.insert_before(MessageBus::Rack::Middleware, RailsFailover::ActiveRecord::Middleware) end diff --git a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb b/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb deleted file mode 100644 index 7cc4dce92bd..00000000000 --- a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb +++ /dev/null @@ -1,190 +0,0 @@ -# frozen_string_literal: true - -require 'active_record/connection_adapters/abstract_adapter' -require 'active_record/connection_adapters/postgresql_adapter' -require 'discourse' -require 'sidekiq/pausable' - -class PostgreSQLFallbackHandler - include Singleton - - attr_reader :masters_down - attr_accessor :initialized - - DATABASE_DOWN_CHANNEL = '/global/database_down' - - def initialize - @masters_down = DistributedCache.new('masters_down', namespace: false) - @mutex = Mutex.new - @initialized = false - - MessageBus.subscribe(DATABASE_DOWN_CHANNEL) do |payload| - if @initialized && payload.data["pid"].to_i != Process.pid - begin - RailsMultisite::ConnectionManagement.with_connection(payload.data['db']) do - clear_connections - end - rescue PG::UnableToSend - # Site has already failed over - end - end - end - end - - def verify_master - synchronize do - return if @thread && @thread.alive? - - @thread = Thread.new do - while true do - thread = Thread.new { initiate_fallback_to_master } - thread.abort_on_exception = true - thread.join - break if synchronize { @masters_down.hash.empty? } - sleep 5 - end - end - - @thread.abort_on_exception = true - end - end - - def master_down? - synchronize { @masters_down[namespace] } - end - - def master_down - synchronize do - @masters_down[namespace] = true - Sidekiq.pause!("pg_failover") if !Sidekiq.paused? - MessageBus.publish(DATABASE_DOWN_CHANNEL, db: namespace, pid: Process.pid) - end - end - - def master_up(namespace) - synchronize { @masters_down.delete(namespace, publish: false) } - end - - def initiate_fallback_to_master - begin - unless @initialized - @initialized = true - return - end - - @masters_down.hash.keys.each do |key| - RailsMultisite::ConnectionManagement.with_connection(key) do - begin - logger.warn "#{log_prefix}: Checking master server..." - is_connection_active = false - - begin - connection = ActiveRecord::Base.postgresql_connection(config) - is_connection_active = connection.active? - ensure - connection.disconnect! if connection - end - - if is_connection_active - logger.warn "#{log_prefix}: Master server is active. Reconnecting..." - self.master_up(key) - clear_connections - disable_readonly_mode - Sidekiq.unpause! - end - rescue => e - logger.warn "#{log_prefix}: Connection to master PostgreSQL server failed with '#{e.message}'" - end - end - end - rescue => e - logger.warn "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" - end - end - - # Use for testing - def setup! - @masters_down.clear - disable_readonly_mode - end - - def clear_connections - ActiveRecord::Base.clear_all_connections! - end - - private - - def disable_readonly_mode - Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) - end - - def config - ActiveRecord::Base.connection_config - end - - def logger - Rails.logger - end - - def log_prefix - "#{self.class} [#{namespace}]" - end - - def namespace - RailsMultisite::ConnectionManagement.current_db - end - - def synchronize - @mutex.synchronize { yield } - end -end - -module ActiveRecord - module ConnectionHandling - def postgresql_fallback_connection(config) - return postgresql_connection(config) if ARGV.include?("db:migrate") - fallback_handler = ::PostgreSQLFallbackHandler.instance - config = config.symbolize_keys - - if fallback_handler.master_down? - Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) - fallback_handler.verify_master - connection = replica_postgresql_connection(config) - else - begin - connection = postgresql_connection(config) - fallback_handler.initialized ||= true - rescue ::ActiveRecord::NoDatabaseError, PG::ConnectionBad => e - fallback_handler.master_down - fallback_handler.verify_master - - if !fallback_handler.initialized - return postgresql_fallback_connection(config) - else - raise e - end - end - end - - connection - end - - def replica_postgresql_connection(config) - config = config.dup.merge( - host: config[:replica_host], - port: config[:replica_port] - ) - - connection = postgresql_connection(config) - verify_replica(connection) - connection - end - - private - - def verify_replica(connection) - value = connection.exec_query("SELECT pg_is_in_recovery()").rows[0][0] - raise "Replica database server is not in recovery mode." if !value - end - end -end diff --git a/lib/discourse.rb b/lib/discourse.rb index 2382acb2c43..bcdcecc8003 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -428,19 +428,21 @@ module Discourse alias_method :base_url_no_path, :base_url_no_prefix end - READONLY_MODE_KEY_TTL ||= 60 - READONLY_MODE_KEY ||= 'readonly_mode' - PG_READONLY_MODE_KEY ||= 'readonly_mode:postgres' - USER_READONLY_MODE_KEY ||= 'readonly_mode:user' + READONLY_MODE_KEY_TTL ||= 60 + READONLY_MODE_KEY ||= 'readonly_mode' + PG_READONLY_MODE_KEY ||= 'readonly_mode:postgres' + USER_READONLY_MODE_KEY ||= 'readonly_mode:user' + PG_FORCE_READONLY_MODE_KEY ||= 'readonly_mode:postgres_force' READONLY_KEYS ||= [ READONLY_MODE_KEY, PG_READONLY_MODE_KEY, - USER_READONLY_MODE_KEY + USER_READONLY_MODE_KEY, + PG_FORCE_READONLY_MODE_KEY ] def self.enable_readonly_mode(key = READONLY_MODE_KEY) - if key == USER_READONLY_MODE_KEY + if key == USER_READONLY_MODE_KEY || key == PG_FORCE_READONLY_MODE_KEY Discourse.redis.set(key, 1) else Discourse.redis.setex(key, READONLY_MODE_KEY_TTL, 1) @@ -486,6 +488,24 @@ module Discourse true end + def self.enable_pg_force_readonly_mode + RailsMultisite::ConnectionManagement.each_connection do + enable_readonly_mode(PG_FORCE_READONLY_MODE_KEY) + Sidekiq.pause!("pg_failover") if !Sidekiq.paused? + end + + true + end + + def self.disable_pg_force_readonly_mode + RailsMultisite::ConnectionManagement.each_connection do + disable_readonly_mode(PG_FORCE_READONLY_MODE_KEY) + Sidekiq.unpause! + end + + true + end + def self.readonly_mode?(keys = READONLY_KEYS) recently_readonly? || Discourse.redis.exists?(*keys) end diff --git a/lib/discourse_redis.rb b/lib/discourse_redis.rb index 53a04905f37..33390d4ca6f 100644 --- a/lib/discourse_redis.rb +++ b/lib/discourse_redis.rb @@ -5,141 +5,6 @@ # class DiscourseRedis - class FallbackHandler - include Singleton - - MASTER_ROLE_STATUS = "role:master" - MASTER_LOADING_STATUS = "loading:1" - MASTER_LOADED_STATUS = "loading:0" - CONNECTION_TYPES = %w{normal pubsub} - - def initialize - @master = true - @running = false - @mutex = Mutex.new - @slave_config = DiscourseRedis.slave_config - @message_bus_keepalive_interval = MessageBus.keepalive_interval - end - - def verify_master - synchronize do - return if @thread && @thread.alive? - - @thread = Thread.new do - loop do - begin - thread = Thread.new { initiate_fallback_to_master } - thread.join - break if synchronize { @master } - sleep 5 - ensure - thread.kill - end - end - end - end - end - - def initiate_fallback_to_master - success = false - - begin - redis_config = DiscourseRedis.config.dup - redis_config.delete(:connector) - master_client = ::Redis::Client.new(redis_config) - logger.warn "#{log_prefix}: Checking connection to master server..." - info = master_client.call([:info]) - - if info.include?(MASTER_LOADED_STATUS) && info.include?(MASTER_ROLE_STATUS) - begin - logger.warn "#{log_prefix}: Master server is active, killing all connections to slave..." - - self.master = true - slave_client = ::Redis::Client.new(@slave_config) - - CONNECTION_TYPES.each do |connection_type| - slave_client.call([:client, [:kill, 'type', connection_type]]) - end - - MessageBus.keepalive_interval = @message_bus_keepalive_interval - Discourse.clear_readonly! - Discourse.request_refresh! - success = true - ensure - slave_client&.disconnect - end - end - rescue => e - logger.warn "#{log_prefix}: Connection to Master server failed with '#{e.message}'" - ensure - master_client&.disconnect - end - - success - end - - def master - synchronize { @master } - end - - def master=(args) - synchronize do - @master = args - - # Disables MessageBus keepalive when Redis is in readonly mode - MessageBus.keepalive_interval = 0 if !@master - end - end - - private - - def synchronize - @mutex.synchronize { yield } - end - - def logger - Rails.logger - end - - def log_prefix - "#{self.class}" - end - end - - class Connector < Redis::Client::Connector - def initialize(options) - super(options) - @slave_options = DiscourseRedis.slave_config(options) - @fallback_handler = DiscourseRedis::FallbackHandler.instance - end - - def resolve(client = nil) - if !@fallback_handler.master - @fallback_handler.verify_master - return @slave_options - end - - begin - options = @options.dup - options.delete(:connector) - client ||= Redis::Client.new(options) - - loading = client.call([:info, :persistence]).include?( - DiscourseRedis::FallbackHandler::MASTER_LOADING_STATUS - ) - - loading ? @slave_options : @options - rescue Redis::ConnectionError, Redis::CannotConnectError, RuntimeError => ex - raise ex if ex.class == RuntimeError && ex.message != "Name or service not known" - @fallback_handler.master = false - @fallback_handler.verify_master - raise ex - ensure - client.disconnect - end - end - end - def self.raw_connection(config = nil) config ||= self.config Redis.new(config) @@ -149,20 +14,12 @@ class DiscourseRedis GlobalSetting.redis_config end - def self.slave_config(options = config) - options.dup.merge!(host: options[:slave_host], port: options[:slave_port]) - end - def initialize(config = nil, namespace: true) @config = config || DiscourseRedis.config @redis = DiscourseRedis.raw_connection(@config.dup) @namespace = namespace end - def self.fallback_handler - @fallback_handler ||= DiscourseRedis::FallbackHandler.instance - end - def without_namespace # Only use this if you want to store and fetch data that's shared between sites @redis @@ -172,10 +29,6 @@ class DiscourseRedis yield rescue Redis::CommandError => ex if ex.message =~ /READONLY/ - if !ENV["REDIS_RAILS_FAILOVER"] - fallback_handler.verify_master if !fallback_handler.master - end - Discourse.received_redis_readonly! nil else diff --git a/lib/freedom_patches/zeitwerk.rb b/lib/freedom_patches/zeitwerk.rb index cf1ddc186d3..92a066b8339 100644 --- a/lib/freedom_patches/zeitwerk.rb +++ b/lib/freedom_patches/zeitwerk.rb @@ -9,7 +9,6 @@ module ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector 'onpdiff' => 'ONPDiff', 'onceoff' => 'Jobs', 'pop3_polling_enabled_setting_validator' => 'POP3PollingEnabledSettingValidator', - 'postgresql_fallback_adapter' => 'PostgreSQLFallbackHandler', 'regular' => 'Jobs', 'scheduled' => 'Jobs', 'topic_query_sql' => 'TopicQuerySQL', diff --git a/spec/models/global_setting_spec.rb b/spec/models/global_setting_spec.rb index d923d6e31d6..88024182773 100644 --- a/spec/models/global_setting_spec.rb +++ b/spec/models/global_setting_spec.rb @@ -86,7 +86,7 @@ describe GlobalSetting do GlobalSetting.expects(:redis_slave_port).returns(6379).at_least_once GlobalSetting.expects(:redis_slave_host).returns('0.0.0.0').at_least_once - expect(GlobalSetting.redis_config[:connector]).to eq(DiscourseRedis::Connector) + expect(GlobalSetting.redis_config[:connector]).to eq(RailsFailover::Redis::Connector) end end end