From 4cd6c91d5f1f3e58e2f2a58d16691d0651f24a7e Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 10 Sep 2019 14:09:55 +0100 Subject: Redis set cache docs and minor cleanup --- lib/gitlab/repository_cache_adapter.rb | 22 +++++++++++++++++----- lib/gitlab/repository_set_cache.rb | 4 ++-- 2 files changed, 19 insertions(+), 7 deletions(-) (limited to 'lib/gitlab') diff --git a/lib/gitlab/repository_cache_adapter.rb b/lib/gitlab/repository_cache_adapter.rb index 75503ee1789..b2dc92ce010 100644 --- a/lib/gitlab/repository_cache_adapter.rb +++ b/lib/gitlab/repository_cache_adapter.rb @@ -35,6 +35,11 @@ module Gitlab # name - The name of the method to be cached. # fallback - A value to fall back to if the repository does not exist, or # in case of a Git error. Defaults to nil. + # + # It is not safe to use this method prior to the release of 12.3, since + # 12.2 does not correctly invalidate the redis set cache value. A mixed + # code environment containing both 12.2 and 12.3 nodes breaks, while a + # mixed code environment containing both 12.3 and 12.4 nodes will work. def cache_method_as_redis_set(name, fallback: nil) uncached_name = alias_uncached_method(name) @@ -44,13 +49,20 @@ module Gitlab end end + # Attempt to determine whether a value is in the set of values being + # cached, by performing a redis SISMEMBERS query if appropriate. + # + # If the full list is already in-memory, we're better using it directly. + # + # If the cache is not yet populated, querying it directly will give the + # wrong answer. We handle that by querying the full list - which fills + # the cache - and using it directly to answer the question. define_method("#{name}_include?") do |value| - # If the cache isn't populated, we can't rely on it - return redis_set_cache.include?(name, value) if redis_set_cache.exist?(name) + if strong_memoized?(name) || !redis_set_cache.exist?(name) + return __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend + end - # Since we have to pull all branch names to populate the cache, use - # the data we already have to answer the query just this once - __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend + redis_set_cache.include?(name, value) end end diff --git a/lib/gitlab/repository_set_cache.rb b/lib/gitlab/repository_set_cache.rb index fb634328a95..6d3ac53a787 100644 --- a/lib/gitlab/repository_set_cache.rb +++ b/lib/gitlab/repository_set_cache.rb @@ -13,7 +13,7 @@ module Gitlab end def cache_key(type) - [type, namespace, 'set'].join(':') + "#{type}:#{namespace}:set" end def expire(key) @@ -37,7 +37,7 @@ module Gitlab # Splitting into groups of 1000 prevents us from creating a too-long # Redis command - value.in_groups_of(1000, false) { |subset| redis.sadd(full_key, subset) } + value.each_slice(1000) { |subset| redis.sadd(full_key, subset) } redis.expire(full_key, expires_in) end -- cgit v1.2.3