From 2cbb513c9806f3aa9459f524c27aa5c6809ff724 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Sat, 15 Dec 2018 08:53:52 +0800 Subject: [PATCH] FIX: Don't use `Redis#keys` in production. As per the documentation for KEYS ``` Warning: consider KEYS as a command that should only be used in production environments with extreme care. It may ruin performance when it is executed against large databases. This command is intended for debugging and special operations, such as changing your keyspace layout. ``` Instead SCAN ``` Since these commands allow for incremental iteration, returning only a small number of elements per call, they can be used in production without the downside of commands like KEYS or SMEMBERS that may block the server for a long time (even several seconds) when called against big collections of keys or elements. ``` --- lib/cache.rb | 2 +- lib/discourse_redis.rb | 31 +++++++++++++++++++++++++ spec/components/discourse_redis_spec.rb | 7 ++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/cache.rb b/lib/cache.rb index 597ae2ce1dd..2f3689a02ca 100644 --- a/lib/cache.rb +++ b/lib/cache.rb @@ -22,7 +22,7 @@ class Cache < ActiveSupport::Cache::Store end def keys(pattern = "*") - redis.keys("#{@namespace}:#{pattern}") + redis.scan_each(match: "#{@namespace}:#{pattern}").to_a end def clear diff --git a/lib/discourse_redis.rb b/lib/discourse_redis.rb index 00647242b84..2a67c0515f0 100644 --- a/lib/discourse_redis.rb +++ b/lib/discourse_redis.rb @@ -201,6 +201,31 @@ class DiscourseRedis end end + def scan_each(options = {}, &block) + DiscourseRedis.ignore_readonly do + match = options[:match].presence || '*' + + options[:match] = + if @namespace + "#{namespace}:#{match}" + else + match + end + + if block + @redis.scan_each(options) do |key| + key = remove_namespace(key) if @namespace + block.call(key) + end + else + @redis.scan_each(options).map do |key| + key = remove_namespace(key) if @namespace + key + end + end + end + end + def keys(pattern = nil) DiscourseRedis.ignore_readonly do pattern = pattern || '*' @@ -253,4 +278,10 @@ class DiscourseRedis Cache.new end + private + + def remove_namespace(key) + key[(namespace.length + 1)..-1] + end + end diff --git a/spec/components/discourse_redis_spec.rb b/spec/components/discourse_redis_spec.rb index 7e87438d70f..a99c763414b 100644 --- a/spec/components/discourse_redis_spec.rb +++ b/spec/components/discourse_redis_spec.rb @@ -35,15 +35,22 @@ describe DiscourseRedis do expect(redis.keys).to include('key') expect(redis.keys).to_not include('key2') + expect(redis.scan_each.to_a).to eq(['key']) + + redis.scan_each.each do |key| + expect(key).to eq('key') + end redis.del('key') expect(raw_redis.get('default:key')).to eq(nil) + expect(redis.scan_each.to_a).to eq([]) raw_redis.set('default:key1', '1') raw_redis.set('default:key2', '2') expect(redis.mget('key1', 'key2')).to eq(['1', '2']) + expect(redis.scan_each.to_a).to contain_exactly('key1', 'key2') end end