Allow caching of negative FindCommit matches
When FindCommit ref caching is enabled, negative matches would previously not be cached. However, if a source branch is deleted, there's no need to keep looking up the same commit. This change caches the result of a nil commit.
This commit is contained in:
parent
8db4a54df0
commit
52215e8983
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Allow caching of negative FindCommit matches
|
||||
merge_request: 29952
|
||||
author:
|
||||
type: performance
|
|
@ -271,26 +271,30 @@ module Gitlab
|
|||
end
|
||||
|
||||
def find_commit(revision)
|
||||
if Gitlab::SafeRequestStore.active?
|
||||
# We don't use Gitlab::SafeRequestStore.fetch(key) { ... } directly
|
||||
# because `revision` can be a branch name, so we can't use it as a key
|
||||
# as it could point to another commit later on (happens a lot in
|
||||
# tests).
|
||||
key = {
|
||||
storage: @gitaly_repo.storage_name,
|
||||
relative_path: @gitaly_repo.relative_path,
|
||||
commit_id: revision
|
||||
}
|
||||
return Gitlab::SafeRequestStore[key] if Gitlab::SafeRequestStore.exist?(key)
|
||||
return call_find_commit(revision) unless Gitlab::SafeRequestStore.active?
|
||||
|
||||
commit = call_find_commit(revision)
|
||||
return unless commit
|
||||
# We don't use Gitlab::SafeRequestStore.fetch(key) { ... } directly
|
||||
# because `revision` can be a branch name, so we can't use it as a key
|
||||
# as it could point to another commit later on (happens a lot in
|
||||
# tests).
|
||||
key = {
|
||||
storage: @gitaly_repo.storage_name,
|
||||
relative_path: @gitaly_repo.relative_path,
|
||||
commit_id: revision
|
||||
}
|
||||
return Gitlab::SafeRequestStore[key] if Gitlab::SafeRequestStore.exist?(key)
|
||||
|
||||
key[:commit_id] = commit.id unless GitalyClient.ref_name_caching_allowed?
|
||||
commit = call_find_commit(revision)
|
||||
|
||||
if GitalyClient.ref_name_caching_allowed?
|
||||
Gitlab::SafeRequestStore[key] = commit
|
||||
else
|
||||
call_find_commit(revision)
|
||||
return commit
|
||||
end
|
||||
|
||||
return unless commit
|
||||
|
||||
key[:commit_id] = commit.id
|
||||
Gitlab::SafeRequestStore[key] = commit
|
||||
end
|
||||
|
||||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
|
|
|
@ -223,6 +223,19 @@ describe Gitlab::GitalyClient::CommitService do
|
|||
end
|
||||
|
||||
context 'when caching of the ref name is enabled' do
|
||||
it 'caches negative entries' do
|
||||
expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit).once.and_return(double(commit: nil))
|
||||
|
||||
commit = nil
|
||||
2.times do
|
||||
::Gitlab::GitalyClient.allow_ref_name_caching do
|
||||
commit = described_class.new(repository).find_commit('master')
|
||||
end
|
||||
end
|
||||
|
||||
expect(commit).to eq(nil)
|
||||
end
|
||||
|
||||
it 'returns a cached commit' do
|
||||
expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit).once.and_return(double(commit: commit_dbl))
|
||||
|
||||
|
|
Loading…
Reference in New Issue