FIX: Don't use DistributedCache to store redis readonly state

This can cause unbound CPU usage in some cases, and excessive logging in other cases. This commit moves redis readonly information into the local process, but maintains the DistributedCache for postgres readonly state.
This commit is contained in:
David Taylor 2019-06-21 15:08:57 +01:00 committed by Guo Xiang Tan
parent f3e4e6941c
commit afb5ec811d
7 changed files with 49 additions and 20 deletions

View File

@ -139,7 +139,7 @@ class ApplicationController < ActionController::Base
end
rescue_from PG::ReadOnlySqlTransaction do |e|
Discourse.received_readonly!
Discourse.received_postgres_readonly!
Rails.logger.error("#{e.class} #{e.message}: #{e.backtrace.join("\n")}")
raise Discourse::ReadOnly
end

View File

@ -377,21 +377,34 @@ module Discourse
$redis.get(PG_READONLY_MODE_KEY).present?
end
def self.last_read_only
@last_read_only ||= DistributedCache.new('last_read_only', namespace: false)
# Shared between processes
def self.postgres_last_read_only
@postgres_last_read_only ||= DistributedCache.new('postgres_last_read_only', namespace: false)
end
# Per-process
def self.redis_last_read_only
@redis_last_read_only ||= {}
end
def self.recently_readonly?
read_only = last_read_only[$redis.namespace]
read_only.present? && read_only > 15.seconds.ago
postgres_read_only = postgres_last_read_only[$redis.namespace]
redis_read_only = redis_last_read_only[$redis.namespace]
(redis_read_only.present? && redis_read_only > 15.seconds.ago) ||
(postgres_read_only.present? && postgres_read_only > 15.seconds.ago)
end
def self.received_readonly!
last_read_only[$redis.namespace] = Time.zone.now
def self.received_postgres_readonly!
postgres_last_read_only[$redis.namespace] = Time.zone.now
end
def self.received_redis_readonly!
redis_last_read_only[$redis.namespace] = Time.zone.now
end
def self.clear_readonly!
last_read_only[$redis.namespace] = nil
postgres_last_read_only[$redis.namespace] = redis_last_read_only[$redis.namespace] = nil
Site.clear_anon_cache!
true
end
@ -667,7 +680,11 @@ module Discourse
if !$redis.without_namespace.get(redis_key)
Rails.logger.warn(warning)
$redis.without_namespace.setex(redis_key, 3600, "x")
begin
$redis.without_namespace.setex(redis_key, 3600, "x")
rescue Redis::CommandError => e
raise unless e.message =~ /READONLY/
end
end
warning
end

View File

@ -176,7 +176,7 @@ class DiscourseRedis
end
fallback_handler.verify_master if !fallback_handler.master
Discourse.received_readonly!
Discourse.received_redis_readonly!
nil
else
raise ex

View File

@ -219,8 +219,13 @@ describe Discourse do
expect(Discourse.readonly_mode?).to eq(true)
end
it "returns true when Discourse is recently read only" do
Discourse.received_readonly!
it "returns true when postgres is recently read only" do
Discourse.received_postgres_readonly!
expect(Discourse.readonly_mode?).to eq(true)
end
it "returns true when redis is recently read only" do
Discourse.received_redis_readonly!
expect(Discourse.readonly_mode?).to eq(true)
end
@ -234,21 +239,28 @@ describe Discourse do
end
end
describe ".received_readonly!" do
describe ".received_postgres_readonly!" do
it "sets the right time" do
time = Discourse.received_readonly!
expect(Discourse.last_read_only['default']).to eq(time)
time = Discourse.received_postgres_readonly!
expect(Discourse.postgres_last_read_only['default']).to eq(time)
end
end
describe ".received_redis_readonly!" do
it "sets the right time" do
time = Discourse.received_redis_readonly!
expect(Discourse.redis_last_read_only['default']).to eq(time)
end
end
describe ".clear_readonly!" do
it "publishes the right message" do
Discourse.received_readonly!
Discourse.received_postgres_readonly!
messages = []
expect do
messages = MessageBus.track_publish { Discourse.clear_readonly! }
end.to change { Discourse.last_read_only['default'] }.to(nil)
end.to change { Discourse.postgres_last_read_only['default'] }.to(nil)
expect(messages.any? { |m| m.channel == Site::SITE_JSON_CHANNEL })
.to eq(true)

View File

@ -12,7 +12,7 @@ RSpec.describe ForumsController do
end
it "returns a readonly header if the site is read only" do
Discourse.received_readonly!
Discourse.received_postgres_readonly!
get "/srv/status"
expect(response.status).to eq(200)
expect(response.headers['Discourse-Readonly']).to eq('true')

View File

@ -1762,7 +1762,7 @@ RSpec.describe TopicsController do
end
it "returns a readonly header if the site is read only" do
Discourse.received_readonly!
Discourse.received_postgres_readonly!
get "/t/#{topic.id}.json"
expect(response.status).to eq(200)
expect(response.headers['Discourse-Readonly']).to eq('true')

View File

@ -16,7 +16,7 @@ describe RandomTopicSelector do
expect(RandomTopicSelector.next(0)).to eq([])
expect(RandomTopicSelector.next(2)).to eq([0, 1])
$redis.expects(:multi).returns(Discourse.received_readonly!)
$redis.expects(:multi).returns(Discourse.received_redis_readonly!)
expect(RandomTopicSelector.next(2)).to eq([2, 3])
$redis.unstub(:multi)