Commit 625d8231 authored by Kerri Miller's avatar Kerri Miller Committed by Alex Kalderimis

Cache calls to Repository#merge_to_ref

It stands to reason that for a given target/source sha combination,
subsequent calls should result in the same result.. so why call it more
than once for a given combination (especially since this can be a slow,
weighty call to make to Gitaly?)
parent 83fe069e
...@@ -66,6 +66,16 @@ module MergeRequests ...@@ -66,6 +66,16 @@ module MergeRequests
end end
def commit def commit
if Feature.enabled?(:cache_merge_to_ref_calls, project, default_enabled: false)
Rails.cache.fetch(cache_key, expires_in: 1.day) do
extracted_merge_to_ref
end
else
extracted_merge_to_ref
end
end
def extracted_merge_to_ref
repository.merge_to_ref(current_user, repository.merge_to_ref(current_user,
source_sha: source, source_sha: source,
branch: merge_request.target_branch, branch: merge_request.target_branch,
...@@ -76,5 +86,9 @@ module MergeRequests ...@@ -76,5 +86,9 @@ module MergeRequests
rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => error rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => error
raise MergeError, error.message raise MergeError, error.message
end end
def cache_key
[:merge_to_ref_service, project.full_path, merge_request.target_branch_sha, merge_request.source_branch_sha]
end
end end
end end
---
name: cache_merge_to_ref_calls
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67789
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338251
milestone: '14.2'
type: development
group: group::code review
default_enabled: false
...@@ -36,6 +36,37 @@ RSpec.describe MergeRequests::MergeToRefService do ...@@ -36,6 +36,37 @@ RSpec.describe MergeRequests::MergeToRefService do
expect(repository.ref_exists?(target_ref)).to be(true) expect(repository.ref_exists?(target_ref)).to be(true)
expect(ref_head.id).to eq(result[:commit_id]) expect(ref_head.id).to eq(result[:commit_id])
end end
context 'cache_merge_to_ref_calls flag enabled', :use_clean_rails_memory_store_caching do
before do
stub_feature_flags(cache_merge_to_ref_calls: true)
# warm the cache
#
service.execute(merge_request)
end
it 'caches the response', :request_store do
expect { 3.times { service.execute(merge_request) } }
.not_to change(Gitlab::GitalyClient, :get_request_count)
end
end
context 'cache_merge_to_ref_calls flag disabled', :use_clean_rails_memory_store_caching do
before do
stub_feature_flags(cache_merge_to_ref_calls: false)
# warm the cache
#
service.execute(merge_request)
end
it 'does not cache the response', :request_store do
expect(Gitlab::GitalyClient).to receive(:call).at_least(3).times.and_call_original
3.times { service.execute(merge_request) }
end
end
end end
shared_examples_for 'successfully evaluates pre-condition checks' do shared_examples_for 'successfully evaluates pre-condition checks' 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