Merge branch '43109-ci_environments_status-json-executes-more-than-100-queries' into 'master'
Resolve "Controller Projects::MergeRequestsController#ci_environments_status.json executes more than 100 SQL queries" Closes #43109 See merge request gitlab-org/gitlab-ce!21996
This commit is contained in:
commit
81e435c4a5
4 changed files with 52 additions and 29 deletions
|
@ -217,7 +217,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
|
||||||
|
|
||||||
metrics_url =
|
metrics_url =
|
||||||
if can?(current_user, :read_environment, environment) && environment.has_metrics?
|
if can?(current_user, :read_environment, environment) && environment.has_metrics?
|
||||||
metrics_project_environment_deployment_path(environment.project, environment, deployment)
|
metrics_project_environment_deployment_path(project, environment, deployment)
|
||||||
end
|
end
|
||||||
|
|
||||||
metrics_monitoring_url =
|
metrics_monitoring_url =
|
||||||
|
|
|
@ -1082,26 +1082,10 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_or_initialize_services(exceptions: [])
|
def find_or_initialize_services(exceptions: [])
|
||||||
services_templates = Service.where(template: true)
|
|
||||||
|
|
||||||
available_services_names = Service.available_services_names - exceptions
|
available_services_names = Service.available_services_names - exceptions
|
||||||
|
|
||||||
available_services = available_services_names.map do |service_name|
|
available_services = available_services_names.map do |service_name|
|
||||||
service = find_service(services, service_name)
|
find_or_initialize_service(service_name)
|
||||||
|
|
||||||
if service
|
|
||||||
service
|
|
||||||
else
|
|
||||||
# We should check if template for the service exists
|
|
||||||
template = find_service(services_templates, service_name)
|
|
||||||
|
|
||||||
if template.nil?
|
|
||||||
# If no template, we should create an instance. Ex `build_gitlab_ci_service`
|
|
||||||
public_send("build_#{service_name}_service") # rubocop:disable GitlabSecurity/PublicSend
|
|
||||||
else
|
|
||||||
Service.build_from_template(id, template)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
available_services.reject do |service|
|
available_services.reject do |service|
|
||||||
|
@ -1114,7 +1098,18 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_or_initialize_service(name)
|
def find_or_initialize_service(name)
|
||||||
find_or_initialize_services.find { |service| service.to_param == name }
|
service = find_service(services, name)
|
||||||
|
return service if service
|
||||||
|
|
||||||
|
# We should check if template for the service exists
|
||||||
|
template = find_service(services_templates, name)
|
||||||
|
|
||||||
|
if template
|
||||||
|
Service.build_from_template(id, template)
|
||||||
|
else
|
||||||
|
# If no template, we should create an instance. Ex `build_gitlab_ci_service`
|
||||||
|
public_send("build_#{name}_service") # rubocop:disable GitlabSecurity/PublicSend
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# rubocop: disable CodeReuse/ServiceClass
|
# rubocop: disable CodeReuse/ServiceClass
|
||||||
|
@ -2277,4 +2272,8 @@ class Project < ActiveRecord::Base
|
||||||
check_access.call
|
check_access.call
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def services_templates
|
||||||
|
@services_templates ||= Service.where(template: true)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -772,16 +772,26 @@ describe Projects::MergeRequestsController do
|
||||||
create(:merge_request, source_project: forked, target_project: project)
|
create(:merge_request, source_project: forked, target_project: project)
|
||||||
end
|
end
|
||||||
|
|
||||||
before do
|
it 'links to the environment on that project' do
|
||||||
|
get_ci_environments_status
|
||||||
|
|
||||||
|
expect(json_response.first['url']).to match /#{forked.full_path}/
|
||||||
|
end
|
||||||
|
|
||||||
|
# we're trying to reduce the overall number of queries for this method.
|
||||||
|
# set a hard limit for now. https://gitlab.com/gitlab-org/gitlab-ce/issues/52287
|
||||||
|
it 'keeps queries in check' do
|
||||||
|
control_count = ActiveRecord::QueryRecorder.new { get_ci_environments_status }.count
|
||||||
|
|
||||||
|
expect(control_count).to be <= 137
|
||||||
|
end
|
||||||
|
|
||||||
|
def get_ci_environments_status
|
||||||
get :ci_environments_status,
|
get :ci_environments_status,
|
||||||
namespace_id: merge_request.project.namespace.to_param,
|
namespace_id: merge_request.project.namespace.to_param,
|
||||||
project_id: merge_request.project,
|
project_id: merge_request.project,
|
||||||
id: merge_request.iid, format: 'json'
|
id: merge_request.iid, format: 'json'
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'links to the environment on that project' do
|
|
||||||
expect(json_response.first['url']).to match /#{forked.full_path}/
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -4037,6 +4037,20 @@ describe Project do
|
||||||
expect(result).to be_empty
|
expect(result).to be_empty
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#find_or_initialize_service" do
|
||||||
|
subject { build(:project) }
|
||||||
|
|
||||||
|
it 'avoids N+1 database queries' do
|
||||||
|
allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover))
|
||||||
|
|
||||||
|
control_count = ActiveRecord::QueryRecorder.new { project.find_or_initialize_service('prometheus') }.count
|
||||||
|
|
||||||
|
allow(Service).to receive(:available_services_names).and_call_original
|
||||||
|
|
||||||
|
expect { project.find_or_initialize_service('prometheus') }.not_to exceed_query_limit(control_count)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def rugged_config
|
def rugged_config
|
||||||
|
|
Loading…
Reference in a new issue