From 1b5ee0ae7246d59195114aa49f536a8c0f9c9bc0 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 17 Oct 2017 13:38:46 +0800 Subject: [PATCH] FIX: Couldn't boot Discourse app with a readonly postgres. --- .../postgresql_fallback_adapter.rb | 34 +++++++++--- .../postgresql_fallback_adapter_spec.rb | 52 ++++++++++++------- 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb b/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb index f151830c5fa..5dc5f2278ec 100644 --- a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb +++ b/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb @@ -1,6 +1,7 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_record/connection_adapters/postgresql_adapter' require 'discourse' +require 'sidekiq/pausable' class PostgreSQLFallbackHandler include Singleton @@ -48,14 +49,15 @@ class PostgreSQLFallbackHandler begin logger.warn "#{log_prefix}: Checking master server..." connection = ActiveRecord::Base.postgresql_connection(config) + is_connection_active = connection.active? + connection.disconnect! - if connection.active? - connection.disconnect! - ActiveRecord::Base.clear_all_connections! + if is_connection_active logger.warn "#{log_prefix}: Master server is active. Reconnecting..." - + ActiveRecord::Base.clear_active_connections! + ActiveRecord::Base.clear_all_connections! self.master_up(key) - Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) + disable_readonly_mode Sidekiq.unpause! end rescue => e @@ -68,10 +70,15 @@ class PostgreSQLFallbackHandler # Use for testing def setup! @masters_down = {} + disable_readonly_mode end private + def disable_readonly_mode + Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) + end + def config ActiveRecord::Base.connection_config end @@ -100,19 +107,30 @@ module ActiveRecord config = config.symbolize_keys if fallback_handler.master_down? + Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) fallback_handler.verify_master - connection = postgresql_connection(config.dup.merge(host: config[:replica_host], port: config[:replica_port])) + connection = postgresql_connection(config.dup.merge( + host: config[:replica_host], + port: config[:replica_port] + )) verify_replica(connection) - Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) else begin + now = Time.zone.now connection = postgresql_connection(config) + fallback_handler.master_down = false rescue PG::ConnectionBad => e + on_boot = fallback_handler.master_down?.nil? fallback_handler.master_down = true fallback_handler.verify_master - raise e + + if on_boot + return postgresql_fallback_connection(config) + else + raise e + end end end diff --git a/spec/components/active_record/connection_adapters/postgresql_fallback_adapter_spec.rb b/spec/components/active_record/connection_adapters/postgresql_fallback_adapter_spec.rb index 0013cf2be50..c0170df7db8 100644 --- a/spec/components/active_record/connection_adapters/postgresql_fallback_adapter_spec.rb +++ b/spec/components/active_record/connection_adapters/postgresql_fallback_adapter_spec.rb @@ -6,13 +6,34 @@ describe ActiveRecord::ConnectionHandling do let(:replica_port) { 6432 } let(:config) do - ActiveRecord::Base.configurations[Rails.env].merge("adapter" => "postgresql_fallback", - "replica_host" => replica_host, - "replica_port" => replica_port).symbolize_keys! + ActiveRecord::Base.configurations[Rails.env].merge( + "adapter" => "postgresql_fallback", + "replica_host" => replica_host, + "replica_port" => replica_port + ).symbolize_keys! + end + + let(:multisite_db) { "database_2" } + + let(:multisite_config) do + { + host: 'localhost1', + port: 5432, + replica_host: replica_host, + replica_port: replica_port + } end let(:postgresql_fallback_handler) { PostgreSQLFallbackHandler.instance } + before do + ['default', multisite_db].each do |db| + with_multisite_db(db) do + postgresql_fallback_handler.master_down = false + end + end + end + after do postgresql_fallback_handler.setup! end @@ -24,17 +45,6 @@ describe ActiveRecord::ConnectionHandling do end context 'when master server is down' do - let(:multisite_db) { "database_2" } - - let(:multisite_config) do - { - host: 'localhost1', - port: 5432, - replica_host: replica_host, - replica_port: replica_port - } - end - before do @replica_connection = mock('replica_connection') end @@ -59,10 +69,12 @@ describe ActiveRecord::ConnectionHandling do ActiveRecord::Base.expects(:postgresql_connection).with(configuration).raises(PG::ConnectionBad) ActiveRecord::Base.expects(:verify_replica).with(@replica_connection) - ActiveRecord::Base.expects(:postgresql_connection).with(configuration.merge(host: replica_host, port: replica_port)).returns(@replica_connection) + ActiveRecord::Base.expects(:postgresql_connection).with(configuration.merge( + host: replica_host, port: replica_port) + ).returns(@replica_connection) end - expect(postgresql_fallback_handler.master_down?).to eq(nil) + expect(postgresql_fallback_handler.master_down?).to eq(false) expect { ActiveRecord::Base.postgresql_fallback_connection(config) } .to raise_error(PG::ConnectionBad) @@ -74,7 +86,7 @@ describe ActiveRecord::ConnectionHandling do expect(Sidekiq.paused?).to eq(true) with_multisite_db(multisite_db) do - expect(postgresql_fallback_handler.master_down?).to eq(nil) + expect(postgresql_fallback_handler.master_down?).to eq(false) expect { ActiveRecord::Base.postgresql_fallback_connection(multisite_config) } .to raise_error(PG::ConnectionBad) @@ -96,7 +108,6 @@ describe ActiveRecord::ConnectionHandling do expect(Sidekiq.paused?).to eq(false) expect(ActiveRecord::Base.connection_pool.connections.count).to eq(0) - skip("Figuring out why the following keeps failing to obtain a connection on Travis") expect(ActiveRecord::Base.connection) .to be_an_instance_of(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter) end @@ -106,7 +117,10 @@ describe ActiveRecord::ConnectionHandling do it 'should raise the right error' do ActiveRecord::Base.expects(:postgresql_connection).with(config).raises(PG::ConnectionBad).once - ActiveRecord::Base.expects(:postgresql_connection).with(config.dup.merge(host: replica_host, port: replica_port)).raises(PG::ConnectionBad).once + ActiveRecord::Base.expects(:postgresql_connection).with(config.dup.merge( + host: replica_host, + port: replica_port + )).raises(PG::ConnectionBad).once 2.times do expect { ActiveRecord::Base.postgresql_fallback_connection(config) }