Commit 88b0461b authored by Alex Kalderimis's avatar Alex Kalderimis

Performance improvements to CI GraphQL resources

This collection of improvements was implemented when working on
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40088,
but can be cleanly extracted here.

It includes N+1 fixes.
parent 1de70204
......@@ -16,6 +16,26 @@ module Types
def detailed_status
object.detailed_status(context[:current_user])
end
# Issues one query per pipeline
def groups
BatchLoader::GraphQL.for([object.pipeline, object]).batch(default_value: []) do |keys, loader|
by_pipeline = keys.group_by(&:first)
by_pipeline.each do |pl, key_group|
project = pl.project
stages = key_group.map(&:second).uniq
indexed = stages.index_by(&:id)
results = pl.latest_statuses.where(stage_id: stages.map(&:id)) # rubocop: disable CodeReuse/ActiveRecord
results.group_by(&:stage_id).each do |stage_id, statuses|
stage = indexed[stage_id]
groups = ::Ci::Group.fabricate(project, stage, statuses)
loader.call([pl, stage], groups)
end
end
end
end
end
end
end
......@@ -39,8 +39,13 @@ module Ci
end
end
def self.fabricate(project, stage)
stage.latest_statuses
# Construct a grouping of statuses for this stage.
# We allow the caller to pass in statuses for efficiency (avoiding N+1
# queries).
def self.fabricate(project, stage, statuses = nil)
statuses ||= stage.latest_statuses
statuses
.sort_by(&:sortable_name).group_by(&:group_name)
.map do |group_name, grouped_statuses|
self.new(project, stage, name: group_name, jobs: grouped_statuses)
......
---
title: Performance improvements for CI GraphQL resources
merge_request: 50386
author:
type: performance
......@@ -9499,11 +9499,6 @@ type GeoNode {
"""
first: Int
"""
Global ID of a specific compliance framework to return.
"""
id: ComplianceManagementFrameworkID
"""
Returns the last _n_ elements from the list.
"""
......@@ -9739,6 +9734,11 @@ type Group {
"""
first: Int
"""
Global ID of a specific compliance framework to return.
"""
id: ComplianceManagementFrameworkID
"""
Returns the last _n_ elements from the list.
"""
......
......@@ -26177,16 +26177,6 @@
"ofType": null
},
"defaultValue": null
},
{
"name": "id",
"description": "Global ID of a specific compliance framework to return.",
"type": {
"kind": "SCALAR",
"name": "ComplianceManagementFrameworkID",
"ofType": null
},
"defaultValue": null
}
],
"type": {
......@@ -26979,6 +26969,16 @@
"ofType": null
},
"defaultValue": null
},
{
"name": "id",
"description": "Global ID of a specific compliance framework to return.",
"type": {
"kind": "SCALAR",
"name": "ComplianceManagementFrameworkID",
"ofType": null
},
"defaultValue": null
}
],
"type": {
......@@ -5,7 +5,8 @@ require 'spec_helper'
RSpec.describe Resolvers::Ci::JobsResolver do
include GraphqlHelpers
let_it_be(:pipeline) { create(:ci_pipeline) }
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
before_all do
create(:ci_build, name: 'Normal job', pipeline: pipeline)
......
......@@ -7,27 +7,33 @@ RSpec.describe 'Query.project.pipeline' do
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:user) { create(:user) }
def first(field)
[field.pluralize, 'nodes', 0]
def all(*fields)
fields.flat_map { |f| [f, :nodes] }
end
describe '.stages.groups.jobs' do
let(:pipeline) do
pipeline = create(:ci_pipeline, project: project, user: user)
stage = create(:ci_stage_entity, pipeline: pipeline, name: 'first')
stage = create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'first')
create(:commit_status, stage_id: stage.id, pipeline: pipeline, name: 'my test job')
pipeline
end
let(:jobs_graphql_data) { graphql_data.dig(*%w[project pipeline], *first('stage'), *first('group'), 'jobs', 'nodes') }
let(:jobs_graphql_data) { graphql_data_at(:project, :pipeline, *all(:stages, :groups, :jobs)) }
let(:first_n) { var('Int') }
let(:query) do
%(
query {
project(fullPath: "#{project.full_path}") {
pipeline(iid: "#{pipeline.iid}") {
stages {
with_signature([first_n], wrap_fields(query_graphql_path([
[:project, { full_path: project.full_path }],
[:pipeline, { iid: pipeline.iid.to_s }],
[:stages, { first: first_n }]
], stage_fields)))
end
let(:stage_fields) do
<<~FIELDS
nodes {
name
groups {
......@@ -44,11 +50,7 @@ RSpec.describe 'Query.project.pipeline' do
}
}
}
}
}
}
}
)
FIELDS
end
it 'returns the jobs of a pipeline stage' do
......@@ -57,60 +59,43 @@ RSpec.describe 'Query.project.pipeline' do
expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job'))
end
it 'avoids N+1 queries', :aggregate_failures do
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: user)
end
build_stage = create(:ci_stage_entity, name: 'build', pipeline: pipeline)
test_stage = create(:ci_stage_entity, name: 'test', pipeline: pipeline)
describe 'performance' do
before do
build_stage = create(:ci_stage_entity, position: 2, name: 'build', project: project, pipeline: pipeline)
test_stage = create(:ci_stage_entity, position: 3, name: 'test', project: project, pipeline: pipeline)
create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2')
create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2')
create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2')
create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2')
end
expect do
post_graphql(query, current_user: user)
end.not_to exceed_query_limit(control_count)
expect(response).to have_gitlab_http_status(:ok)
it 'can find the first stage' do
post_graphql(query, current_user: user, variables: first_n.with(1))
build_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage|
stage['name'] == 'build'
end
test_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage|
stage['name'] == 'test'
expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job'))
end
docker_group = build_stage.dig('groups', 'nodes').first
rspec_group = test_stage.dig('groups', 'nodes').first
expect(docker_group['name']).to eq('docker')
expect(rspec_group['name']).to eq('rspec')
it 'can find all stages' do
post_graphql(query, current_user: user, variables: first_n.with(3))
expect(jobs_graphql_data).to contain_exactly(
a_hash_including('name' => 'my test job'),
a_hash_including('name' => 'docker 1 2'),
a_hash_including('name' => 'docker 2 2'),
a_hash_including('name' => 'rspec 1 2'),
a_hash_including('name' => 'rspec 2 2')
)
end
docker_jobs = docker_group.dig('jobs', 'nodes')
rspec_jobs = rspec_group.dig('jobs', 'nodes')
it 'avoids N+1 queries' do
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: user, variables: first_n.with(1))
end
expect(docker_jobs).to eq([
{
'name' => 'docker 1 2',
'pipeline' => { 'id' => pipeline.to_global_id.to_s }
},
{
'name' => 'docker 2 2',
'pipeline' => { 'id' => pipeline.to_global_id.to_s }
}
])
expect(rspec_jobs).to eq([
{
'name' => 'rspec 1 2',
'pipeline' => { 'id' => pipeline.to_global_id.to_s }
},
{
'name' => 'rspec 2 2',
'pipeline' => { 'id' => pipeline.to_global_id.to_s }
}
])
expect do
post_graphql(query, current_user: user, variables: first_n.with(3))
end.not_to exceed_query_limit(control_count)
end
end
end
......
......@@ -6,53 +6,59 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
include GraphqlHelpers
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:first_user) { create(:user) }
let_it_be(:second_user) { create(:user) }
let_it_be(:user) { create(:user) }
describe '.jobs' do
let_it_be(:query) do
%(
query {
project(fullPath: "#{project.full_path}") {
pipelines {
nodes {
jobs {
nodes {
name
}
}
}
}
}
}
)
let(:first_n) { var('Int') }
let(:query_path) do
[
[:project, { full_path: project.full_path }],
[:pipelines, { first: first_n }],
[:nodes],
[:jobs],
[:nodes]
]
end
it 'fetches the jobs without an N+1' do
pipeline = create(:ci_pipeline, project: project)
create(:ci_build, pipeline: pipeline, name: 'Job 1')
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: first_user)
let(:query) do
with_signature([first_n], wrap_fields(query_graphql_path(query_path, :name)))
end
before_all do
pipeline = create(:ci_pipeline, project: project)
create(:ci_build, pipeline: pipeline, name: 'Job 1')
pipeline = create(:ci_pipeline, project: project)
create(:ci_build, pipeline: pipeline, name: 'Job 2')
end
expect do
post_graphql(query, current_user: second_user)
end.not_to exceed_query_limit(control_count)
it 'limits the results' do
post_graphql(query, current_user: user, variables: first_n.with(1))
expect(response).to have_gitlab_http_status(:ok)
expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly a_hash_including(
'name' => 'Job 2'
)
end
pipelines_data = graphql_data.dig('project', 'pipelines', 'nodes')
it 'fetches all results' do
post_graphql(query, current_user: user)
job_names = pipelines_data.map do |pipeline_data|
jobs_data = pipeline_data.dig('jobs', 'nodes')
jobs_data.map { |job_data| job_data['name'] }
end.flatten
expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly(
a_hash_including('name' => 'Job 1'),
a_hash_including('name' => 'Job 2')
)
end
expect(job_names).to contain_exactly('Job 1', 'Job 2')
it 'fetches the jobs without an N+1' do
first_user = create(:personal_access_token).user
second_user = create(:personal_access_token).user
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: first_user, variables: first_n.with(1))
end
expect do
post_graphql(query, current_user: second_user)
end.not_to exceed_query_limit(control_count)
end
end
......@@ -80,7 +86,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
create(:ci_build, :dast, name: 'DAST Job 1', pipeline: pipeline)
create(:ci_build, :sast, name: 'SAST Job 1', pipeline: pipeline)
post_graphql(query, current_user: first_user)
post_graphql(query, current_user: user)
expect(response).to have_gitlab_http_status(:ok)
......@@ -96,9 +102,9 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
end
describe 'upstream' do
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
let_it_be(:upstream_project) { create(:project, :repository, :public) }
let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: first_user) }
let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: user) }
let(:upstream_pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]).first['upstream'] }
let(:query) do
......@@ -120,7 +126,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
before do
create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline )
post_graphql(query, current_user: first_user)
post_graphql(query, current_user: user)
end
it_behaves_like 'a working graphql query'
......@@ -131,15 +137,18 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
context 'when fetching the upstream pipeline from the pipeline' do
it 'avoids N+1 queries' do
first_user = create(:user)
second_user = create(:user)
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: first_user)
end
pipeline_2 = create(:ci_pipeline, project: project, user: first_user)
upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: first_user)
pipeline_2 = create(:ci_pipeline, project: project, user: user)
upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: user)
create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2 )
pipeline_3 = create(:ci_pipeline, project: project, user: first_user)
upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: first_user)
pipeline_3 = create(:ci_pipeline, project: project, user: user)
upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: user)
create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3 )
expect do
......@@ -152,12 +161,12 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
end
describe 'downstream' do
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) }
let(:pipeline_2) { create(:ci_pipeline, project: project, user: first_user) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
let(:pipeline_2) { create(:ci_pipeline, project: project, user: user) }
let_it_be(:downstream_project) { create(:project, :repository, :public) }
let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: first_user) }
let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: first_user) }
let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: user) }
let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: user) }
let(:pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]) }
......@@ -183,7 +192,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_a)
create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_b)
post_graphql(query, current_user: first_user)
post_graphql(query, current_user: user)
end
it_behaves_like 'a working graphql query'
......@@ -198,16 +207,19 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
context 'when fetching the downstream pipelines from the pipeline' do
it 'avoids N+1 queries' do
first_user = create(:user)
second_user = create(:user)
control_count = ActiveRecord::QueryRecorder.new do
post_graphql(query, current_user: first_user)
end
downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: first_user)
downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: user)
create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_2a)
downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: first_user)
downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: user)
create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downsteam_pipeline_3a)
downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: first_user)
downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: user)
create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_2b)
downsteam_pipeline_3b = create(:ci_pipeline, project: downstream_project, user: first_user)
create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downsteam_pipeline_3b)
......
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