Merge branch '331319-new-repository-set-cache-key-expiry-only' into 'master'

Use consistent cache naming for set caches: Expire new key format

See merge request gitlab-org/gitlab!62264

lib/gitlab/reactive_cache_set_cache.rb

@@ -11,12 +11,16 @@ module Gitlab
     end
 
     def cache_key(key)
-      "#{cache_type}:#{key}:set"
+      "#{cache_namespace}:#{key}:set"
+    end
+
+    def new_cache_key(key)
+      super(key)
     end
 
     def clear_cache!(key)
       with do |redis|
-        keys = read(key).map { |value| "#{cache_type}:#{value}" }
+        keys = read(key).map { |value| "#{cache_namespace}:#{value}" }
         keys << cache_key(key)
 
         redis.pipelined do
@@ -24,11 +28,5 @@ module Gitlab
         end
       end
     end
-
-    private
-
-    def cache_type
-      Gitlab::Redis::Cache::CACHE_NAMESPACE
-    end
   end
 end

lib/gitlab/repository_set_cache.rb

@@ -17,6 +17,11 @@ module Gitlab
       "#{type}:#{namespace}:set"
     end
 
+    # NOTE Remove as part of #331319
+    def new_cache_key(type)
+      super("#{type}:#{namespace}")
+    end
+
     def write(key, value)
       full_key = cache_key(key)
 

lib/gitlab/set_cache.rb

@@ -14,15 +14,21 @@ module Gitlab
       "#{key}:set"
     end
 
+    # NOTE Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/331319
+    def new_cache_key(key)
+      "#{cache_namespace}:#{key}:set"
+    end
+
     # Returns the number of keys deleted by Redis
     def expire(*keys)
       return 0 if keys.empty?
 
       with do |redis|
-        keys = keys.map { |key| cache_key(key) }
+        keys_to_expire = keys.map { |key| cache_key(key) }
+        keys_to_expire += keys.map { |key| new_cache_key(key) } # NOTE Remove as part of #331319
 
         Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-          redis.unlink(*keys)
+          redis.unlink(*keys_to_expire)
         end
       end
     end
@@ -73,5 +79,9 @@ module Gitlab
     def with(&blk)
       Gitlab::Redis::Cache.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
     end
+
+    def cache_namespace
+      Gitlab::Redis::Cache::CACHE_NAMESPACE
+    end
   end
 end

spec/lib/gitlab/repository_set_cache_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
 
   let(:repository) { project.repository }
   let(:namespace) { "#{repository.full_path}:#{project.id}" }
+  let(:gitlab_cache_namespace) { Gitlab::Redis::Cache::CACHE_NAMESPACE }
   let(:cache) { described_class.new(repository) }
 
   describe '#cache_key' do
@@ -52,6 +53,24 @@ RSpec.describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
     end
   end
 
+  describe '#write' do
+    subject(:write_cache) { cache.write('branch_names', ['main']) }
+
+    it 'writes the value to the cache' do
+      write_cache
+
+      redis_keys = Gitlab::Redis::Cache.with { |redis| redis.scan(0, match: "*") }.last
+      expect(redis_keys).to include("branch_names:#{namespace}:set")
+      expect(cache.fetch('branch_names')).to contain_exactly('main')
+    end
+
+    it 'sets the expiry of the set' do
+      write_cache
+
+      expect(cache.ttl('branch_names')).to be_within(1).of(cache.expires_in.seconds)
+    end
+  end
+
   describe '#expire' do
     subject { cache.expire(*keys) }
 
@@ -75,6 +94,12 @@ RSpec.describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
 
         expect(cache.read(:foo)).to be_empty
       end
+
+      it 'expires the new key format' do
+        expect_any_instance_of(Redis).to receive(:unlink).with(cache.cache_key(:foo), cache.new_cache_key(:foo)) # rubocop:disable RSpec/AnyInstanceOf
+
+        subject
+      end
     end
 
     context 'multiple keys' do

spec/tasks/cache/clear/redis_spec.rb

@@ -2,11 +2,15 @@
 
 require 'rake_helper'
 
-RSpec.describe 'clearing redis cache' do
+RSpec.describe 'clearing redis cache', :clean_gitlab_redis_cache do
   before do
     Rake.application.rake_require 'tasks/cache'
   end
 
+  shared_examples 'clears the cache' do
+    it { expect { run_rake_task('cache:clear:redis') }.to change { redis_keys.size }.by(-1) }
+  end
+
   describe 'clearing pipeline status cache' do
     let(:pipeline_status) do
       project = create(:project, :repository)
@@ -20,5 +24,38 @@ RSpec.describe 'clearing redis cache' do
     it 'clears pipeline status cache' do
       expect { run_rake_task('cache:clear:redis') }.to change { pipeline_status.has_cache? }
     end
+
+    it_behaves_like 'clears the cache'
+  end
+
+  describe 'clearing set caches' do
+    context 'repository set' do
+      let(:project) { create(:project) }
+      let(:repository) { project.repository }
+
+      let(:cache) { Gitlab::RepositorySetCache.new(repository) }
+
+      before do
+        pending "Enable as part of https://gitlab.com/gitlab-org/gitlab/-/issues/331319"
+
+        cache.write(:foo, [:bar])
+      end
+
+      it_behaves_like 'clears the cache'
+    end
+
+    context 'reactive cache set' do
+      let(:cache) { Gitlab::ReactiveCacheSetCache.new }
+
+      before do
+        cache.write(:foo, :bar)
+      end
+
+      it_behaves_like 'clears the cache'
+    end
+  end
+
+  def redis_keys
+    Gitlab::Redis::Cache.with { |redis| redis.scan(0, match: "*") }.last
   end
 end