Merge branch 'bvl-cache-repository-ancestor' into 'master'

Cache Repository#ancestor? on two levels

See merge request gitlab-org/gitlab!20958

app/models/repository.rb

@@ -925,7 +925,22 @@ class Repository
   def ancestor?(ancestor_id, descendant_id)
     return false if ancestor_id.nil? || descendant_id.nil?
 
-    raw_repository.ancestor?(ancestor_id, descendant_id)
+    counter = Gitlab::Metrics.counter(
+      :repository_ancestor_calls_total,
+      'The number of times we call Repository#ancestor with valid arguments')
+    cache_hit = true
+
+    cache_key = "ancestor:#{ancestor_id}:#{descendant_id}"
+    result = request_store_cache.fetch(cache_key) do
+      cache.fetch(cache_key) do
+        cache_hit = false
+        raw_repository.ancestor?(ancestor_id, descendant_id)
+      end
+    end
+
+    counter.increment(cache_hit: cache_hit.to_s)
+
+    result
   end
 
   def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true)

changelogs/unreleased/bvl-cache-repository-ancestor.yml

+---
+title: Cache the ancestor? Gitaly call to speed up polling for the merge request widget
+merge_request: 20958
+author:
+type: performance

spec/models/repository_spec.rb

@@ -2399,7 +2399,40 @@ describe Repository do
   end
 
   describe '#ancestor? with Gitaly enabled' do
-    it_behaves_like "#ancestor?"
+    let(:commit) { repository.commit }
+    let(:ancestor) { commit.parents.first }
+    let(:cache_key) { "ancestor:#{ancestor.id}:#{commit.id}" }
+
+    it_behaves_like '#ancestor?'
+
+    context 'caching', :request_store, :clean_gitlab_redis_cache do
+      it 'only calls out to Gitaly once' do
+        expect(repository.raw_repository).to receive(:ancestor?).once
+
+        2.times { repository.ancestor?(commit.id, ancestor.id) }
+      end
+
+      it 'increments a counter with cache hits' do
+        counter = Gitlab::Metrics.counter(:repository_ancestor_calls_total, 'Repository ancestor calls')
+
+        expect do
+          2.times { repository.ancestor?(commit.id, ancestor.id) }
+        end.to change { counter.get(cache_hit: 'true') }.by(1)
+                 .and change { counter.get(cache_hit: 'false') }.by(1)
+      end
+
+      it 'returns the value from the request store' do
+        repository.__send__(:request_store_cache).write(cache_key, "it's apparent")
+
+        expect(repository.ancestor?(ancestor.id, commit.id)).to eq("it's apparent")
+      end
+
+      it 'returns the value from the redis cache' do
+        expect(repository.__send__(:cache)).to receive(:fetch).with(cache_key).and_return("it's apparent")
+
+        expect(repository.ancestor?(ancestor.id, commit.id)).to eq("it's apparent")
+      end
+    end
   end
 
   describe '#ancestor? with Rugged enabled', :enable_rugged do