PERF: Reduce number of EXPIRE calls from CachedCounting (#15958)

Previously we were calling `EXPIRE` every time we incremented a given key. Instead, we can call EXPIRE once when the key is first populated. A LUA script is used to make this as efficient as possible.

Consumers of this Concern use daily keys. Since we're now calling EXPIRE only at the beginning of the day, rather than throughout the day, the expire time has been increased from 3 to 4 days.
This commit is contained in:
David Taylor 2022-02-15 16:55:21 +00:00 committed by GitHub
parent 11c93342dc
commit 94a47d037f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 11 deletions

View File

@ -3,6 +3,18 @@
module CachedCounting
extend ActiveSupport::Concern
EXPIRE_CACHE_AFTER = 4.days.to_i
LUA_INCR_AND_EXPIRE = DiscourseRedis::EvalHelper.new <<~LUA
local result = redis.call("INCR", KEYS[1])
if result == 1 then
redis.call("EXPIRE", KEYS[1], ARGV[1])
end
return result
LUA
included do
class << self
attr_accessor :autoflush, :autoflush_seconds, :last_flush
@ -19,13 +31,16 @@ module CachedCounting
class_methods do
def perform_increment!(key, opts = nil)
val = Discourse.redis.incr(key).to_i
val = DiscourseRedis.ignore_readonly do
LUA_INCR_AND_EXPIRE.eval(
Discourse.redis.without_namespace,
[Discourse.redis.namespace_key(key)],
[EXPIRE_CACHE_AFTER]
).to_i
end
# readonly mode it is going to be 0, skip
return if val == 0
# 3.days, see: https://github.com/rails/rails/issues/21296
Discourse.redis.expire(key, 259200)
# readonly mode it is going to be nil, skip
return if val.nil?
autoflush = (opts && opts[:autoflush]) || self.autoflush
if autoflush > 0 && val >= autoflush
@ -45,7 +60,7 @@ module CachedCounting
# this may seem a bit fancy but in so it allows
# for concurrent calls without double counting
def get_and_reset(key)
Discourse.redis.set(key, '0', ex: 259200, get: true).to_i
Discourse.redis.set(key, '0', ex: EXPIRE_CACHE_AFTER, get: true).to_i
end
def request_id(query_params, retries = 0)

View File

@ -29,14 +29,16 @@ describe ApplicationRequest do
inc(:http_total)
inc(:http_total)
Discourse.redis.without_namespace.stubs(:incr).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:eval).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:evalsha).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:set).raises(Redis::CommandError.new("READONLY"))
# flush will be deferred no error raised
inc(:http_total, autoflush: 3)
ApplicationRequest.write_cache!
Discourse.redis.without_namespace.unstub(:incr)
Discourse.redis.without_namespace.unstub(:eval)
Discourse.redis.without_namespace.unstub(:evalsha)
Discourse.redis.without_namespace.unstub(:set)
inc(:http_total, autoflush: 3)

View File

@ -31,13 +31,15 @@ describe WebCrawlerRequest do
inc('Googlebot')
inc('Googlebot')
Discourse.redis.without_namespace.stubs(:incr).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:eval).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:evalsha).raises(Redis::CommandError.new("READONLY"))
Discourse.redis.without_namespace.stubs(:set).raises(Redis::CommandError.new("READONLY"))
inc('Googlebot', autoflush: 3)
WebCrawlerRequest.write_cache!
Discourse.redis.without_namespace.unstub(:incr)
Discourse.redis.without_namespace.unstub(:eval)
Discourse.redis.without_namespace.unstub(:evalsha)
Discourse.redis.without_namespace.unstub(:set)
inc('Googlebot', autoflush: 3)