diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2019-06-26 17:41:06 +0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2019-06-26 17:41:06 +0300 |
commit | 1c571a8f3a71cca480060dfd44c58e8cf951c85e (patch) | |
tree | 21691c61214fcd2dda3b1b85dc664f36654eec33 | |
parent | 50de1ede0d74a4eacf8319af27079d51be24f01c (diff) | |
parent | 52215e8983fe72c9d0c3086a83c94a885b8aa0d3 (diff) |
Merge branch 'sh-cache-negative-entries-find-commit' into 'master'
Allow caching of negative FindCommit matches
See merge request gitlab-org/gitlab-ce!29952
-rw-r--r-- | changelogs/unreleased/sh-cache-negative-entries-find-commit.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit_service.rb | 40 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/commit_service_spec.rb | 13 |
3 files changed, 40 insertions, 18 deletions
diff --git a/changelogs/unreleased/sh-cache-negative-entries-find-commit.yml b/changelogs/unreleased/sh-cache-negative-entries-find-commit.yml new file mode 100644 index 00000000000..98eb13ee620 --- /dev/null +++ b/changelogs/unreleased/sh-cache-negative-entries-find-commit.yml @@ -0,0 +1,5 @@ +--- +title: Allow caching of negative FindCommit matches +merge_request: 29952 +author: +type: performance diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index d21b98d36ea..a80ce462ab0 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -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) - - commit = call_find_commit(revision) - return unless commit - - key[:commit_id] = commit.id unless GitalyClient.ref_name_caching_allowed? + return call_find_commit(revision) unless 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) + + 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 diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 6d6107ca3e7..ba6abba4e61 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -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)) |