Commit 042dd090 authored by Bala Kumar's avatar Bala Kumar

Fix N+1 database queries in pipeline databuilder

Changelog: fixed
parent 521671b1
...@@ -12,7 +12,7 @@ class PipelineHooksWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -12,7 +12,7 @@ class PipelineHooksWorker # rubocop:disable Scalability/IdempotentWorker
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def perform(pipeline_id) def perform(pipeline_id)
Ci::Pipeline.includes({ builds: { runner: :tags } }) Ci::Pipeline
.find_by(id: pipeline_id) .find_by(id: pipeline_id)
.try(:execute_hooks) .try(:execute_hooks)
end end
......
...@@ -19,18 +19,40 @@ module Gitlab ...@@ -19,18 +19,40 @@ module Gitlab
user: pipeline.user.try(:hook_attrs), user: pipeline.user.try(:hook_attrs),
project: pipeline.project.hook_attrs(backward: false), project: pipeline.project.hook_attrs(backward: false),
commit: pipeline.commit.try(:hook_attrs), commit: pipeline.commit.try(:hook_attrs),
builds: Gitlab::Lazy.new { pipeline.builds.latest.map(&method(:build_hook_attrs)) } builds: Gitlab::Lazy.new do
preload_builds(pipeline, :latest_builds)
pipeline.latest_builds.map(&method(:build_hook_attrs))
end
) )
end end
def with_retried_builds def with_retried_builds
merge( merge(
builds: Gitlab::Lazy.new { @pipeline.builds.map(&method(:build_hook_attrs)) } builds: Gitlab::Lazy.new do
preload_builds(@pipeline, :builds)
@pipeline.builds.map(&method(:build_hook_attrs))
end
) )
end end
private private
# rubocop: disable CodeReuse/ActiveRecord
def preload_builds(pipeline, association)
ActiveRecord::Associations::Preloader.new.preload(pipeline,
{
association => {
**::Ci::Pipeline::PROJECT_ROUTE_AND_NAMESPACE_ROUTE,
runner: :tags,
job_artifacts_archive: [],
user: [],
metadata: []
}
}
)
end
# rubocop: enable CodeReuse/ActiveRecord
def hook_attrs(pipeline) def hook_attrs(pipeline)
{ {
id: pipeline.id, id: pipeline.id,
......
...@@ -237,8 +237,13 @@ FactoryBot.define do ...@@ -237,8 +237,13 @@ FactoryBot.define do
# to the job. If `build.deployment` has already been set, it doesn't # to the job. If `build.deployment` has already been set, it doesn't
# build a new instance. # build a new instance.
environment = Gitlab::Ci::Pipeline::Seed::Environment.new(build).to_resource environment = Gitlab::Ci::Pipeline::Seed::Environment.new(build).to_resource
build.deployment =
Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource build.assign_attributes(
deployment: Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource,
metadata_attributes: {
expanded_environment_name: environment.name
}
)
end end
end end
......
...@@ -131,5 +131,41 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do ...@@ -131,5 +131,41 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do
expect(build_environment_data[:deployment_tier]).to eq(build.persisted_environment.try(:tier)) expect(build_environment_data[:deployment_tier]).to eq(build.persisted_environment.try(:tier))
end end
end end
context 'avoids N+1 database queries' do
it "with multiple builds" do
# Preparing the pipeline with the minimal builds
pipeline = create(:ci_pipeline, user: user, project: project)
create(:ci_build, user: user, project: project, pipeline: pipeline)
create(:ci_build, :deploy_to_production, :with_deployment, user: user, project: project, pipeline: pipeline)
# We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).to_json }.count
# Adding more builds to the pipeline and serializing the data again
create_list(:ci_build, 3, user: user, project: project, pipeline: pipeline)
create(:ci_build, :start_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
create(:ci_build, :stop_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
expect { described_class.build(pipeline.reload).to_json }.not_to exceed_query_limit(control_count)
end
it "with multiple retried builds" do
# Preparing the pipeline with the minimal builds
pipeline = create(:ci_pipeline, user: user, project: project)
create(:ci_build, :retried, user: user, project: project, pipeline: pipeline)
create(:ci_build, :deploy_to_production, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
# We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).with_retried_builds.to_json }.count
# Adding more builds to the pipeline and serializing the data again
create_list(:ci_build, 3, :retried, user: user, project: project, pipeline: pipeline)
create(:ci_build, :start_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
create(:ci_build, :stop_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
expect { described_class.build(pipeline.reload).with_retried_builds.to_json }.not_to exceed_query_limit(control_count)
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