Commit 0f7ed337 authored by Stan Hu's avatar Stan Hu

Cache Repository#root_ref within a request

When an empty project is loaded in the UI, there are 15 separate Gitaly
FindDefaultBranch calls to determine the root_ref. Previously, it was
not cached even within the request. This change caches it within the
request so only a single FindDefaultBranch RPC is needed.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58684
parent 121de6dc
...@@ -534,10 +534,9 @@ class Repository ...@@ -534,10 +534,9 @@ class Repository
end end
def root_ref def root_ref
# When the repo does not exist, or there is no root ref, we raise this error so no data is cached. raw_repository&.root_ref
raw_repository&.root_ref or raise Gitlab::Git::Repository::NoRepository # rubocop:disable Style/AndOr
end end
cache_method :root_ref cache_method_asymmetrically :root_ref
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/314 # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/314
def exists? def exists?
......
---
title: Cache Repository#root_ref within a request
merge_request: 25903
author:
type: performance
...@@ -1095,65 +1095,69 @@ describe Repository do ...@@ -1095,65 +1095,69 @@ describe Repository do
end end
end end
describe '#exists?' do shared_examples 'asymmetric cached method' do |method|
it 'returns true when a repository exists' do
expect(repository.exists?).to be(true)
end
it 'returns false if no full path can be constructed' do
allow(repository).to receive(:full_path).and_return(nil)
expect(repository.exists?).to be(false)
end
context 'with broken storage', :broken_storage do
it 'should raise a storage error' do
expect_to_raise_storage_error { broken_repository.exists? }
end
end
context 'asymmetric caching', :use_clean_rails_memory_store_caching, :request_store do context 'asymmetric caching', :use_clean_rails_memory_store_caching, :request_store do
let(:cache) { repository.send(:cache) } let(:cache) { repository.send(:cache) }
let(:request_store_cache) { repository.send(:request_store_cache) } let(:request_store_cache) { repository.send(:request_store_cache) }
context 'when it returns true' do context 'when it returns true' do
before do before do
expect(repository.raw_repository).to receive(:exists?).once.and_return(true) expect(repository.raw_repository).to receive(method).once.and_return(true)
end end
it 'caches the output in RequestStore' do it 'caches the output in RequestStore' do
expect do expect do
repository.exists? repository.send(method)
end.to change { request_store_cache.read(:exists?) }.from(nil).to(true) end.to change { request_store_cache.read(method) }.from(nil).to(true)
end end
it 'caches the output in RepositoryCache' do it 'caches the output in RepositoryCache' do
expect do expect do
repository.exists? repository.send(method)
end.to change { cache.read(:exists?) }.from(nil).to(true) end.to change { cache.read(method) }.from(nil).to(true)
end end
end end
context 'when it returns false' do context 'when it returns false' do
before do before do
expect(repository.raw_repository).to receive(:exists?).once.and_return(false) expect(repository.raw_repository).to receive(method).once.and_return(false)
end end
it 'caches the output in RequestStore' do it 'caches the output in RequestStore' do
expect do expect do
repository.exists? repository.send(method)
end.to change { request_store_cache.read(:exists?) }.from(nil).to(false) end.to change { request_store_cache.read(method) }.from(nil).to(false)
end end
it 'does NOT cache the output in RepositoryCache' do it 'does NOT cache the output in RepositoryCache' do
expect do expect do
repository.exists? repository.send(method)
end.not_to change { cache.read(:exists?) }.from(nil) end.not_to change { cache.read(method) }.from(nil)
end end
end end
end end
end end
describe '#exists?' do
it 'returns true when a repository exists' do
expect(repository.exists?).to be(true)
end
it 'returns false if no full path can be constructed' do
allow(repository).to receive(:full_path).and_return(nil)
expect(repository.exists?).to be(false)
end
context 'with broken storage', :broken_storage do
it 'should raise a storage error' do
expect_to_raise_storage_error { broken_repository.exists? }
end
end
it_behaves_like 'asymmetric cached method', :exists?
end
describe '#has_visible_content?' do describe '#has_visible_content?' do
before do before do
# If raw_repository.has_visible_content? gets called more than once then # If raw_repository.has_visible_content? gets called more than once then
...@@ -1271,6 +1275,8 @@ describe Repository do ...@@ -1271,6 +1275,8 @@ describe Repository do
repository.root_ref repository.root_ref
repository.root_ref repository.root_ref
end end
it_behaves_like 'asymmetric cached method', :root_ref
end end
describe '#expire_root_ref_cache' do describe '#expire_root_ref_cache' 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