Commit d6c57146 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'sh-optimize-locks-check-ce' into 'master'

Eliminate N+1 queries in LFS file lock checks during a push

See merge request gitlab-org/gitlab-ce!19978
parents 57a2ac18 4f9068df
...@@ -25,6 +25,7 @@ class Project < ActiveRecord::Base ...@@ -25,6 +25,7 @@ class Project < ActiveRecord::Base
include FastDestroyAll::Helpers include FastDestroyAll::Helpers
include WithUploads include WithUploads
include BatchDestroyDependentAssociations include BatchDestroyDependentAssociations
extend Gitlab::Cache::RequestCache
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
...@@ -2013,6 +2014,11 @@ class Project < ActiveRecord::Base ...@@ -2013,6 +2014,11 @@ class Project < ActiveRecord::Base
@gitlab_deploy_token ||= deploy_tokens.gitlab_deploy_token @gitlab_deploy_token ||= deploy_tokens.gitlab_deploy_token
end end
def any_lfs_file_locks?
lfs_file_locks.any?
end
request_cache(:any_lfs_file_locks?) { self.id }
private private
def storage def storage
......
---
title: Eliminate N+1 queries in LFS file locks checks during a push
merge_request:
author:
type: performance
...@@ -37,7 +37,7 @@ module Gitlab ...@@ -37,7 +37,7 @@ module Gitlab
def validate_lfs_file_locks? def validate_lfs_file_locks?
strong_memoize(:validate_lfs_file_locks) do strong_memoize(:validate_lfs_file_locks) do
project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev project.lfs_enabled? && newrev && oldrev && project.any_lfs_file_locks?
end end
end end
......
...@@ -934,6 +934,22 @@ describe Gitlab::GitAccess do ...@@ -934,6 +934,22 @@ describe Gitlab::GitAccess do
expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect(project.repository).to receive(:clean_stale_repository_files).and_call_original
expect { push_access_check }.not_to raise_error expect { push_access_check }.not_to raise_error
end end
it 'avoids N+1 queries', :request_store do
# Run this once to establish a baseline. Cached queries should get
# cached, so that when we introduce another change we shouldn't see
# additional queries.
access.check('git-receive-pack', changes)
control_count = ActiveRecord::QueryRecorder.new do
access.check('git-receive-pack', changes)
end
changes = ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature']
# There is still an N+1 query with protected branches
expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1)
end
end end
end end
......
...@@ -2339,6 +2339,22 @@ describe Project do ...@@ -2339,6 +2339,22 @@ describe Project do
end end
end end
describe '#any_lfs_file_locks?', :request_store do
set(:project) { create(:project) }
it 'returns false when there are no LFS file locks' do
expect(project.any_lfs_file_locks?).to be_falsey
end
it 'returns a cached true when there are LFS file locks' do
create(:lfs_file_lock, project: project)
expect(project.lfs_file_locks).to receive(:any?).once.and_call_original
2.times { expect(project.any_lfs_file_locks?).to be_truthy }
end
end
describe '#protected_for?' do describe '#protected_for?' do
let(:project) { create(:project) } let(:project) { create(:project) }
......
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