diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index 84750ecc695..e141e151e1f 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -48,6 +48,8 @@ class GlobalSetting end token end + rescue Redis::CommandError => e + @safe_secret_key_base = SecureRandom.hex(64) if e.message =~ /READONLY/ end def self.load_defaults diff --git a/config/initializers/006-mini_profiler.rb b/config/initializers/006-mini_profiler.rb index a04c888fceb..8a0ceb8a1a8 100644 --- a/config/initializers/006-mini_profiler.rb +++ b/config/initializers/006-mini_profiler.rb @@ -19,7 +19,9 @@ if defined?(Rack::MiniProfiler) # raw_connection means results are not namespaced # # namespacing gets complex, cause mini profiler is in the rack chain way before multisite - Rack::MiniProfiler.config.storage_instance = Rack::MiniProfiler::RedisStore.new(connection: DiscourseRedis.raw_connection) + Rack::MiniProfiler.config.storage_instance = Rack::MiniProfiler::RedisStore.new( + connection: DiscourseRedis.new(nil, namespace: false) + ) skip = [ /^\/message-bus/, diff --git a/lib/discourse_redis.rb b/lib/discourse_redis.rb index f646cba58e8..bffba0860af 100644 --- a/lib/discourse_redis.rb +++ b/lib/discourse_redis.rb @@ -135,9 +135,10 @@ class DiscourseRedis options.dup.merge!(host: options[:slave_host], port: options[:slave_port]) end - def initialize(config = nil) + def initialize(config = nil, namespace: true) @config = config || DiscourseRedis.config @redis = DiscourseRedis.raw_connection(@config) + @namespace = namespace end def self.fallback_handler @@ -183,29 +184,35 @@ class DiscourseRedis :sunion, :ttl, :type, :watch, :zadd, :zcard, :zcount, :zincrby, :zrange, :zrangebyscore, :zrank, :zrem, :zremrangebyrank, :zremrangebyscore, :zrevrange, :zrevrangebyscore, :zrevrank, :zrangebyscore].each do |m| define_method m do |*args| - args[0] = "#{namespace}:#{args[0]}" + args[0] = "#{namespace}:#{args[0]}" if @namespace DiscourseRedis.ignore_readonly { @redis.send(m, *args) } end end def mget(*args) - args.map! { |a| "#{namespace}:#{a}" } + args.map! { |a| "#{namespace}:#{a}" } if @namespace DiscourseRedis.ignore_readonly { @redis.mget(*args) } end def del(k) DiscourseRedis.ignore_readonly do - k = "#{namespace}:#{k}" + k = "#{namespace}:#{k}" if @namespace @redis.del k end end def keys(pattern = nil) DiscourseRedis.ignore_readonly do - len = namespace.length + 1 - @redis.keys("#{namespace}:#{pattern || '*'}").map { - |k| k[len..-1] - } + pattern = pattern || '*' + pattern = "#{namespace}:#{pattern}" if @namespace + keys = @redis.keys(pattern) + + if @namespace + len = namespace.length + 1 + keys.map! { |k| k[len..-1] } + end + + keys end end diff --git a/spec/components/discourse_redis_spec.rb b/spec/components/discourse_redis_spec.rb index a6568427ef9..471752965a7 100644 --- a/spec/components/discourse_redis_spec.rb +++ b/spec/components/discourse_redis_spec.rb @@ -10,6 +10,70 @@ describe DiscourseRedis do let(:fallback_handler) { DiscourseRedis::FallbackHandler.instance } + describe 'redis commands' do + let(:raw_redis) { Redis.new(DiscourseRedis.config) } + + before do + raw_redis.flushall + end + + after do + raw_redis.flushall + end + + describe 'when namespace is enabled' do + let(:redis) { DiscourseRedis.new } + + it 'should append namespace to the keys' do + redis.set('key', 1) + + expect(raw_redis.get('default:key')).to eq('1') + expect(redis.keys).to eq(['key']) + + redis.del('key') + + expect(raw_redis.get('default:key')).to eq(nil) + + raw_redis.set('default:key1', '1') + raw_redis.set('default:key2', '2') + + expect(redis.mget('key1', 'key2')).to eq(['1', '2']) + end + end + + describe 'when namespace is disabled' do + let(:redis) { DiscourseRedis.new(nil, namespace: false) } + + it 'should not append any namespace to the keys' do + redis.set('key', 1) + + expect(raw_redis.get('key')).to eq('1') + expect(redis.keys).to eq(['key']) + + redis.del('key') + + expect(raw_redis.get('key')).to eq(nil) + + raw_redis.set('key1', '1') + raw_redis.set('key2', '2') + + expect(redis.mget('key1', 'key2')).to eq(['1', '2']) + end + + it 'should noop a readonly redis' do + expect(Discourse.recently_readonly?).to eq(false) + + redis.without_namespace + .expects(:set) + .raises(Redis::CommandError.new("READONLY")) + + redis.set('key', 1) + + expect(Discourse.recently_readonly?).to eq(true) + end + end + end + context '.slave_host' do it 'should return the right config' do slave_config = DiscourseRedis.slave_config(config) @@ -22,9 +86,9 @@ describe DiscourseRedis do it 'should check the status of the master server' do begin fallback_handler.master = false - $redis.without_namespace.expects(:get).raises(Redis::CommandError.new("READONLY")) + $redis.without_namespace.expects(:set).raises(Redis::CommandError.new("READONLY")) fallback_handler.expects(:verify_master).once - $redis.get('test') + $redis.set('test', '1') ensure fallback_handler.master = true end