Don't use the redis set cache yet

For zero-downtime deployed in a mixed code environment between 12.2 and
12.3, the branch and tag name cache is incorrectly invalidated - a push
to an old machine will not clear the redis set version of the cache on
the new machine.

This commit ensures that, in 12.3, both set and non-set versions of the
cache are invalidated, but does not write or consult the set version of
the cache. . In 12.4, it will be safe to switch branch and tag names to
the redis set cache both it and the legacy cache will be invalidated
appropriately in such a mixed code environment.

This delays the full implementation of the feature by one release, but
in the absence of a credible feature-flagging strategy, and amidst an
abundance of caution about the effects of too-eager cache expiration, I
believe this is the best approach available to us.

3 files changed, 15 insertions, 50 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 6ed89ed2c32..e5a83366776 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -239,13 +239,13 @@ class Repository
   def branch_exists?(branch_name)
     return false unless raw_repository
 
-    branch_names_include?(branch_name)
+    branch_names.include?(branch_name)
   end
 
   def tag_exists?(tag_name)
     return false unless raw_repository
 
-    tag_names_include?(tag_name)
+    tag_names.include?(tag_name)
   end
 
   def ref_exists?(ref)
@@ -569,10 +569,10 @@ class Repository
   end
 
   delegate :branch_names, to: :raw_repository
-  cache_method_as_redis_set :branch_names, fallback: []
+  cache_method :branch_names, fallback: []
 
   delegate :tag_names, to: :raw_repository
-  cache_method_as_redis_set :tag_names, fallback: []
+  cache_method :tag_names, fallback: []
 
   delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository
   cache_method :branch_count, fallback: 0
diff --git a/changelogs/unreleased/64251-branch-name-set-cache.yml b/changelogs/unreleased/64251-branch-name-set-cache.yml
deleted file mode 100644
index 6ce4bdf5e43..00000000000
--- a/changelogs/unreleased/64251-branch-name-set-cache.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Cache branch and tag names as Redis sets
-merge_request: 30476
-author:
-type: performance
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 79395fcc994..419e1dc2459 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1223,66 +1223,36 @@ describe Repository do
   end
 
   describe '#branch_exists?' do
-    let(:branch) { repository.root_ref }
+    it 'uses branch_names' do
+      allow(repository).to receive(:branch_names).and_return(['foobar'])
 
-    subject { repository.branch_exists?(branch) }
-
-    it 'delegates to branch_names when the cache is empty' do
-      repository.expire_branches_cache
-
-      expect(repository).to receive(:branch_names).and_call_original
-      is_expected.to eq(true)
-    end
-
-    it 'uses redis set caching when the cache is filled' do
-      repository.branch_names # ensure the branch name cache is filled
-
-      expect(repository)
-        .to receive(:branch_names_include?)
-        .with(branch)
-        .and_call_original
-
-      is_expected.to eq(true)
+      expect(repository.branch_exists?('foobar')).to eq(true)
+      expect(repository.branch_exists?('master')).to eq(false)
     end
   end
 
   describe '#tag_exists?' do
-    let(:tag) { repository.tags.first.name }
-
-    subject { repository.tag_exists?(tag) }
-
-    it 'delegates to tag_names when the cache is empty' do
-      repository.expire_tags_cache
-
-      expect(repository).to receive(:tag_names).and_call_original
-      is_expected.to eq(true)
-    end
-
-    it 'uses redis set caching when the cache is filled' do
-      repository.tag_names # ensure the tag name cache is filled
-
-      expect(repository)
-        .to receive(:tag_names_include?)
-        .with(tag)
-        .and_call_original
+    it 'uses tag_names' do
+      allow(repository).to receive(:tag_names).and_return(['foobar'])
 
-      is_expected.to eq(true)
+      expect(repository.tag_exists?('foobar')).to eq(true)
+      expect(repository.tag_exists?('master')).to eq(false)
     end
   end
 
-  describe '#branch_names', :clean_gitlab_redis_cache do
+  describe '#branch_names', :use_clean_rails_memory_store_caching do
     let(:fake_branch_names) { ['foobar'] }
 
     it 'gets cached across Repository instances' do
       allow(repository.raw_repository).to receive(:branch_names).once.and_return(fake_branch_names)
 
-      expect(repository.branch_names).to match_array(fake_branch_names)
+      expect(repository.branch_names).to eq(fake_branch_names)
 
       fresh_repository = Project.find(project.id).repository
       expect(fresh_repository.object_id).not_to eq(repository.object_id)
 
       expect(fresh_repository.raw_repository).not_to receive(:branch_names)
-      expect(fresh_repository.branch_names).to match_array(fake_branch_names)
+      expect(fresh_repository.branch_names).to eq(fake_branch_names)
     end
   end