Commit 9ed56bb0 authored by Stan Hu's avatar Stan Hu

Fix N+1 SQL queries with protected environments

When viewing a specific pipeline, we check whether the user has
permission to deploy a build by calling
`build.project.protected_environment_accessible_to?`. Previously this
led to one SQL query per build in the pipeline.

To fix this, we cache protected environments by name during a request.

Closes https://gitlab.com/gitlab-org/gitlab/issues/55346
parent 80dd64cb
...@@ -589,8 +589,12 @@ module EE ...@@ -589,8 +589,12 @@ module EE
def protected_environment_by_name(environment_name) def protected_environment_by_name(environment_name)
return unless protected_environments_feature_available? return unless protected_environments_feature_available?
key = "protected_environment_by_name:#{id}:#{environment_name}"
::Gitlab::SafeRequestStore.fetch(key) do
protected_environments.find_by(name: environment_name) protected_environments.find_by(name: environment_name)
end end
end
override :after_import override :after_import
def after_import def after_import
......
---
title: Fix N+1 SQL queries with protected environments
merge_request: 22101
author:
type: performance
...@@ -1448,7 +1448,7 @@ describe Project do ...@@ -1448,7 +1448,7 @@ describe Project do
end end
describe '#protected_environment_by_name' do describe '#protected_environment_by_name' do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
subject { project.protected_environment_by_name('production') } subject { project.protected_environment_by_name('production') }
...@@ -1465,19 +1465,27 @@ describe Project do ...@@ -1465,19 +1465,27 @@ describe Project do
context 'when Protected Environments feature is available on the project' do context 'when Protected Environments feature is available on the project' do
let(:feature_available) { true } let(:feature_available) { true }
let(:environment) { create(:environment, name: 'production') }
let(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
context 'when the project environment exists' do context 'when the project environment does not exists' do
before do it { is_expected.to be_nil }
protected_environment
end end
context 'when the project environment exists' do
let_it_be(:environment) { create(:environment, name: 'production') }
let_it_be(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
it { is_expected.to eq(protected_environment) } it { is_expected.to eq(protected_environment) }
end
context 'when the project environment does not exists' do it 'caches environment name', :request_store do
it { is_expected.to be_nil } control_count = ActiveRecord::QueryRecorder.new { project.protected_environment_by_name(protected_environment.name) }
expect do
2.times { project.protected_environment_by_name(protected_environment.name) }
end.not_to exceed_query_limit(control_count)
expect(project.protected_environment_by_name('non-existent-env')).to be_nil
expect(project.protected_environment_by_name(protected_environment.name)).to eq(protected_environment)
end
end end
end end
end end
......
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