From e8a30431297a5a908e5bd2fc4e63e515412efa60 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 10 Nov 2016 23:44:51 +0800
Subject: [PATCH 1/2] Spawn a single thread that checks for PostgreSQL
fallback.
---
.../postgresql_fallback_adapter.rb | 126 +++++++-----------
lib/discourse.rb | 10 +-
.../postgresql_fallback_adapter_spec.rb | 31 ++---
3 files changed, 64 insertions(+), 103 deletions(-)
diff --git a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb b/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb
index b52a6c12ac0..03932d6cc75 100644
--- a/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb
+++ b/lib/active_record/connection_adapters/postgresql_fallback_adapter.rb
@@ -6,23 +6,46 @@ class PostgreSQLFallbackHandler
include Singleton
def initialize
- @master = {}
- @running = {}
- @mutex = {}
- @last_check = {}
-
- setup!
+ @masters_down = {}
+ @mutex = Mutex.new
end
def verify_master
- @mutex[namespace].synchronize do
- return if running || recently_checked?
- @running[namespace] = true
- end
+ synchronize { return if @thread && @thread.alive? }
- current_namespace = namespace
- Thread.new do
- RailsMultisite::ConnectionManagement.with_connection(current_namespace) do
+ @thread = Thread.new do
+ while true do
+ begin
+ thread = Thread.new { initiate_fallback_to_master }
+ thread.join
+ break if synchronize { @masters_down.empty? }
+ sleep 10
+ ensure
+ thread.kill
+ end
+ end
+ end
+ end
+
+ def master_down?
+ synchronize { @masters_down[namespace] }
+ end
+
+ def master_down=(args)
+ synchronize { @masters_down[namespace] = args }
+ end
+
+ def master_up(namespace)
+ synchronize { @masters_down.delete(namespace) }
+ end
+
+ def running?
+ synchronize { @thread.alive? }
+ end
+
+ def initiate_fallback_to_master
+ @masters_down.keys.each do |key|
+ RailsMultisite::ConnectionManagement.with_connection(key) do
begin
logger.warn "#{log_prefix}: Checking master server..."
connection = ActiveRecord::Base.postgresql_connection(config)
@@ -32,54 +55,20 @@ class PostgreSQLFallbackHandler
ActiveRecord::Base.clear_all_connections!
logger.warn "#{log_prefix}: Master server is active. Reconnecting..."
- if namespace == RailsMultisite::ConnectionManagement::DEFAULT
- ActiveRecord::Base.establish_connection(config)
- else
- RailsMultisite::ConnectionManagement.establish_connection(db: namespace)
- end
-
+ self.master_up(key)
Discourse.disable_readonly_mode
- self.master = true
end
rescue => e
- if e.message.include?("could not connect to server")
- logger.warn "#{log_prefix}: Connection to master PostgreSQL server failed with '#{e.message}'"
- else
- raise e
- end
- ensure
- @mutex[namespace].synchronize do
- @last_check[namespace] = Time.zone.now
- @running[namespace] = false
- end
+ byebug
+ logger.warn "#{log_prefix}: Connection to master PostgreSQL server failed with '#{e.message}'"
end
end
end
end
- def master
- @master[namespace]
- end
-
- def master=(args)
- @master[namespace] = args
- end
-
- def running
- @running[namespace]
- end
-
+ # Use for testing
def setup!
- RailsMultisite::ConnectionManagement.all_dbs.each do |db|
- @master[db] = true
- @running[db] = false
- @mutex[db] = Mutex.new
- @last_check[db] = nil
- end
- end
-
- def verify?
- !master && !running
+ @masters_down = {}
end
private
@@ -96,17 +85,13 @@ class PostgreSQLFallbackHandler
"#{self.class} [#{namespace}]"
end
- def recently_checked?
- if @last_check[namespace]
- Time.zone.now <= (@last_check[namespace] + 5.seconds)
- else
- false
- end
- end
-
def namespace
RailsMultisite::ConnectionManagement.current_db
end
+
+ def synchronize
+ @mutex.synchronize { yield }
+ end
end
module ActiveRecord
@@ -115,7 +100,7 @@ module ActiveRecord
fallback_handler = ::PostgreSQLFallbackHandler.instance
config = config.symbolize_keys
- if fallback_handler.verify?
+ if fallback_handler.master_down?
connection = postgresql_connection(config.dup.merge({
host: config[:replica_host], port: config[:replica_port]
}))
@@ -126,7 +111,8 @@ module ActiveRecord
begin
connection = postgresql_connection(config)
rescue PG::ConnectionBad => e
- fallback_handler.master = false
+ fallback_handler.master_down = true
+ fallback_handler.verify_master
raise e
end
end
@@ -141,20 +127,4 @@ module ActiveRecord
raise "Replica database server is not in recovery mode." if value == 'f'
end
end
-
- module ConnectionAdapters
- class PostgreSQLAdapter
- set_callback :checkout, :before, :switch_back?
-
- private
-
- def fallback_handler
- @fallback_handler ||= ::PostgreSQLFallbackHandler.instance
- end
-
- def switch_back?
- fallback_handler.verify_master if fallback_handler.verify?
- end
- end
- end
end
diff --git a/lib/discourse.rb b/lib/discourse.rb
index 1ac6e1b00d1..f28ad2593cb 100644
--- a/lib/discourse.rb
+++ b/lib/discourse.rb
@@ -228,10 +228,12 @@ module Discourse
def self.keep_readonly_mode
# extend the expiry by 1 minute every 30 seconds
- Thread.new do
- while readonly_mode?
- $redis.expire(READONLY_MODE_KEY, READONLY_MODE_KEY_TTL)
- sleep 30.seconds
+ unless Rails.env.test?
+ Thread.new do
+ while readonly_mode?
+ $redis.expire(READONLY_MODE_KEY, READONLY_MODE_KEY_TTL)
+ sleep 30.seconds
+ end
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 05e6b68cd9e..a9e61dbea15 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
@@ -48,8 +48,9 @@ describe ActiveRecord::ConnectionHandling do
end
it 'should failover to a replica server' do
+ current_threads = Thread.list
+
RailsMultisite::ConnectionManagement.stubs(:all_dbs).returns(['default', multisite_db])
- ::PostgreSQLFallbackHandler.instance.setup!
[config, multisite_config].each do |configuration|
ActiveRecord::Base.expects(:postgresql_connection).with(configuration).raises(PG::ConnectionBad)
@@ -60,7 +61,7 @@ describe ActiveRecord::ConnectionHandling do
})).returns(@replica_connection)
end
- expect(postgresql_fallback_handler.master).to eq(true)
+ expect(postgresql_fallback_handler.master_down?).to eq(nil)
expect { ActiveRecord::Base.postgresql_fallback_connection(config) }
.to raise_error(PG::ConnectionBad)
@@ -68,10 +69,10 @@ describe ActiveRecord::ConnectionHandling do
expect{ ActiveRecord::Base.postgresql_fallback_connection(config) }
.to change{ Discourse.readonly_mode? }.from(false).to(true)
- expect(postgresql_fallback_handler.master).to eq(false)
+ expect(postgresql_fallback_handler.master_down?).to eq(true)
with_multisite_db(multisite_db) do
- expect(postgresql_fallback_handler.master).to eq(true)
+ expect(postgresql_fallback_handler.master_down?).to eq(nil)
expect { ActiveRecord::Base.postgresql_fallback_connection(multisite_config) }
.to raise_error(PG::ConnectionBad)
@@ -79,30 +80,18 @@ describe ActiveRecord::ConnectionHandling do
expect{ ActiveRecord::Base.postgresql_fallback_connection(multisite_config) }
.to change{ Discourse.readonly_mode? }.from(false).to(true)
- expect(postgresql_fallback_handler.master).to eq(false)
+ expect(postgresql_fallback_handler.master_down?).to eq(true)
end
+ postgresql_fallback_handler.master_up(multisite_db)
+
ActiveRecord::Base.unstub(:postgresql_connection)
- current_threads = Thread.list
-
- expect{ ActiveRecord::Base.connection_pool.checkout }
- .to change{ Thread.list.size }.by(1)
-
- # Ensure that we don't try to connect back to the replica when a thread
- # is running
- begin
- ActiveRecord::Base.postgresql_fallback_connection(config)
- rescue PG::ConnectionBad => e
- # This is expected if the thread finishes before the above is called.
- end
-
- # Wait for the thread to finish execution
- (Thread.list - current_threads).each(&:join)
+ postgresql_fallback_handler.initiate_fallback_to_master
expect(Discourse.readonly_mode?).to eq(false)
- expect(PostgreSQLFallbackHandler.instance.master).to eq(true)
+ expect(postgresql_fallback_handler.master_down?).to eq(nil)
expect(ActiveRecord::Base.connection_pool.connections.count).to eq(0)
From 759feef3f03c2f2f524b9acbdc10df21a506205c Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Mon, 14 Nov 2016 11:39:19 +0800
Subject: [PATCH 2/2] FIX: No loggers may have been chained.
---
config/initializers/100-logster.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/config/initializers/100-logster.rb b/config/initializers/100-logster.rb
index c39494f6a05..d913051336c 100644
--- a/config/initializers/100-logster.rb
+++ b/config/initializers/100-logster.rb
@@ -87,5 +87,6 @@ RailsMultisite::ConnectionManagement.each_connection do
end
if Rails.configuration.multisite
- Rails.logger.instance_variable_get(:@chained).first.formatter = RailsMultisite::Formatter.new
+ chained = Rails.logger.instance_variable_get(:@chained)
+ chained && chained.first.formatter = RailsMultisite::Formatter.new
end