Commit a028c70b authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Sean McGivern

Cache Repository#ancestor? on two levels

We sometimes call this multiple times per request, and we also call
this from a polled endpoint. So caching it on both levels could
speed up a bunch of those polled requests
parent f709a8e4
...@@ -925,8 +925,23 @@ class Repository ...@@ -925,8 +925,23 @@ class Repository
def ancestor?(ancestor_id, descendant_id) def ancestor?(ancestor_id, descendant_id)
return false if ancestor_id.nil? || descendant_id.nil? return false if ancestor_id.nil? || descendant_id.nil?
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) raw_repository.ancestor?(ancestor_id, descendant_id)
end 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) def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true)
unless remote_name unless remote_name
......
---
title: Cache the ancestor? Gitaly call to speed up polling for the merge request widget
merge_request: 20958
author:
type: performance
...@@ -2399,7 +2399,40 @@ describe Repository do ...@@ -2399,7 +2399,40 @@ describe Repository do
end end
describe '#ancestor? with Gitaly enabled' do 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 end
describe '#ancestor? with Rugged enabled', :enable_rugged do describe '#ancestor? with Rugged enabled', :enable_rugged do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment