Merge branch 'unlink-cache-deletions' into 'master'

Swap to UNLINK for Redis set cache

See merge request gitlab-org/gitlab!27116

changelogs/unreleased/unlink-cache-deletions.yml

+---
+title: Swap to UNLINK for Redis set cache
+merge_request: 27116
+author:
+type: performance

lib/gitlab/repository_cache_adapter.rb

@@ -237,7 +237,7 @@ module Gitlab
     end
 
     def expire_redis_set_method_caches(methods)
-      methods.each { |name| redis_set_cache.expire(name) }
+      redis_set_cache.expire(*methods)
     end
 
     def expire_redis_hash_method_caches(methods)

lib/gitlab/set_cache.rb

@@ -14,8 +14,11 @@ module Gitlab
       "#{key}:set"
     end
 
-    def expire(key)
-      with { |redis| redis.del(cache_key(key)) }
+    def expire(*keys)
+      with do |redis|
+        keys = keys.map { |key| cache_key(key) }
+        unlink_or_delete(redis, keys)
+      end
     end
 
     def exist?(key)
@@ -51,5 +54,15 @@ module Gitlab
     def with(&blk)
       Gitlab::Redis::Cache.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
     end
+
+    def unlink_or_delete(redis, keys)
+      if Feature.enabled?(:repository_set_cache_unlink, default_enabled: true)
+        redis.unlink(*keys)
+      else
+        redis.del(*keys)
+      end
+    rescue ::Redis::CommandError
+      redis.del(*keys)
+    end
   end
 end

spec/lib/gitlab/repository_cache_adapter_spec.rb

@@ -211,8 +211,7 @@ describe Gitlab::RepositoryCacheAdapter do
     it 'expires the caches of the given methods' do
       expect(cache).to receive(:expire).with(:rendered_readme)
       expect(cache).to receive(:expire).with(:branch_names)
-      expect(redis_set_cache).to receive(:expire).with(:rendered_readme)
-      expect(redis_set_cache).to receive(:expire).with(:branch_names)
+      expect(redis_set_cache).to receive(:expire).with(:rendered_readme, :branch_names)
       expect(redis_hash_cache).to receive(:delete).with(:rendered_readme)
       expect(redis_hash_cache).to receive(:delete).with(:branch_names)
 

spec/lib/gitlab/repository_set_cache_spec.rb

@@ -51,14 +51,54 @@ describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
   end
 
   describe '#expire' do
-    it 'expires the given key from the cache' do
+    subject { cache.expire(*keys) }
+
+    before do
       cache.write(:foo, ['value'])
+      cache.write(:bar, ['value2'])
+    end
 
+    it 'actually wrote the values' do
       expect(cache.read(:foo)).to contain_exactly('value')
+      expect(cache.read(:bar)).to contain_exactly('value2')
+    end
+
+    context 'single key' do
+      let(:keys) { %w(foo) }
+
+      it { is_expected.to eq(1) }
+
+      it 'deletes the given key from the cache' do
+        subject
+
+        expect(cache.read(:foo)).to be_empty
+      end
+    end
+
+    context 'multiple keys' do
+      let(:keys) { %w(foo bar) }
+
+      it { is_expected.to eq(2) }
+
+      it 'deletes the given keys from the cache' do
+        subject
+
+        expect(cache.read(:foo)).to be_empty
+        expect(cache.read(:bar)).to be_empty
+      end
+    end
+
+    context "unlink isn't supported" do
+      before do
+        allow_any_instance_of(Redis).to receive(:unlink) { raise ::Redis::CommandError }
+      end
+
+      it 'still deletes the given key' do
         expect(cache.expire(:foo)).to eq(1)
         expect(cache.read(:foo)).to be_empty
       end
     end
+  end
 
   describe '#exist?' do
     it 'checks whether the key exists' do