Merge branch 'backstage/gb/improve-performance-environment-statuses-endpoint' into 'master'
Improve performance of fetching environments statuses Closes #62589 See merge request gitlab-org/gitlab-ce!30560
This commit is contained in:
commit
4d62ef7323
|
@ -40,7 +40,7 @@ class EnvironmentStatus
|
|||
end
|
||||
|
||||
def changes
|
||||
return [] if project.route_map_for(sha).nil?
|
||||
return [] unless has_route_map?
|
||||
|
||||
changed_files.map { |file| build_change(file) }.compact
|
||||
end
|
||||
|
@ -50,6 +50,10 @@ class EnvironmentStatus
|
|||
.merge_request_diff_files.where(deleted_file: false)
|
||||
end
|
||||
|
||||
def has_route_map?
|
||||
project.route_map_for(sha).present?
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
PAGE_EXTENSIONS = /\A\.(s?html?|php|asp|cgi|pl)\z/i.freeze
|
||||
|
|
|
@ -1914,9 +1914,8 @@ class Project < ApplicationRecord
|
|||
@route_maps_by_commit ||= Hash.new do |h, sha|
|
||||
h[sha] = begin
|
||||
data = repository.route_map_for(sha)
|
||||
next unless data
|
||||
|
||||
Gitlab::RouteMap.new(data)
|
||||
Gitlab::RouteMap.new(data) if data
|
||||
rescue Gitlab::RouteMap::FormatError
|
||||
nil
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Improve performance of fetching environments statuses
|
||||
merge_request: 30560
|
||||
author:
|
||||
type: performance
|
|
@ -878,7 +878,7 @@ describe Projects::MergeRequestsController do
|
|||
expect(control_count).to be <= 137
|
||||
end
|
||||
|
||||
it 'has no N+1 issues for environments', :request_store, retry: 0 do
|
||||
it 'has no N+1 SQL issues for environments', :request_store, retry: 0 do
|
||||
# First run to insert test data from lets, which does take up some 30 queries
|
||||
get_ci_environments_status
|
||||
|
||||
|
@ -893,17 +893,64 @@ describe Projects::MergeRequestsController do
|
|||
leeway = 11
|
||||
expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway)
|
||||
end
|
||||
end
|
||||
|
||||
def get_ci_environments_status(extra_params = {})
|
||||
params = {
|
||||
namespace_id: merge_request.project.namespace.to_param,
|
||||
project_id: merge_request.project,
|
||||
id: merge_request.iid,
|
||||
format: 'json'
|
||||
}
|
||||
context 'when a merge request has multiple environments with deployments' do
|
||||
let(:sha) { merge_request.diff_head_sha }
|
||||
let(:ref) { merge_request.source_branch }
|
||||
|
||||
get :ci_environments_status, params: params.merge(extra_params)
|
||||
let!(:build) { create(:ci_build, pipeline: pipeline) }
|
||||
let!(:pipeline) { create(:ci_pipeline, sha: sha, project: project) }
|
||||
let!(:environment) { create(:environment, name: 'env_a', project: project) }
|
||||
let!(:another_environment) { create(:environment, name: 'env_b', project: project) }
|
||||
|
||||
before do
|
||||
merge_request.update_head_pipeline
|
||||
|
||||
create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build)
|
||||
create(:deployment, :succeed, environment: another_environment, sha: sha, ref: ref, deployable: build)
|
||||
end
|
||||
|
||||
it 'exposes multiple environment statuses' do
|
||||
get_ci_environments_status
|
||||
|
||||
expect(json_response.count).to eq 2
|
||||
end
|
||||
|
||||
context 'when route map is not present in the project' do
|
||||
it 'does not have N+1 Gitaly requests for environments', :request_store do
|
||||
expect(merge_request).to be_present
|
||||
|
||||
expect { get_ci_environments_status }
|
||||
.to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when there is route map present in a project' do
|
||||
before do
|
||||
allow_any_instance_of(EnvironmentStatus)
|
||||
.to receive(:has_route_map?)
|
||||
.and_return(true)
|
||||
end
|
||||
|
||||
it 'does not have N+1 Gitaly requests for diff files', :request_store do
|
||||
expect(merge_request.merge_request_diff.merge_request_diff_files).to be_many
|
||||
|
||||
expect { get_ci_environments_status }
|
||||
.to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def get_ci_environments_status(extra_params = {})
|
||||
params = {
|
||||
namespace_id: merge_request.project.namespace.to_param,
|
||||
project_id: merge_request.project,
|
||||
id: merge_request.iid,
|
||||
format: 'json'
|
||||
}
|
||||
|
||||
get :ci_environments_status, params: params.merge(extra_params)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue