Commit 9d18d60b authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '324370-api-jobs-request-the-present_build-likely-has-n-1-problem' into 'master'

Resolve "API: `jobs/request`: the`present_build!` likely has N+1 problem" [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57694
parents b8df7471 7598b7bd
...@@ -165,7 +165,13 @@ module Ci ...@@ -165,7 +165,13 @@ module Ci
end end
def all_dependencies def all_dependencies
dependencies.all if Feature.enabled?(:preload_associations_jobs_request_api_endpoint, project, default_enabled: :yaml)
strong_memoize(:all_dependencies) do
dependencies.all
end
else
dependencies.all
end
end end
private private
......
...@@ -55,6 +55,18 @@ module Ci ...@@ -55,6 +55,18 @@ module Ci
specs specs
end end
# rubocop: disable CodeReuse/ActiveRecord
def all_dependencies
dependencies = super
if Feature.enabled?(:preload_associations_jobs_request_api_endpoint, project, default_enabled: :yaml)
ActiveRecord::Associations::Preloader.new.preload(dependencies, :job_artifacts_archive)
end
dependencies
end
# rubocop: enable CodeReuse/ActiveRecord
private private
def create_archive(artifacts) def create_archive(artifacts)
......
---
name: preload_associations_jobs_request_api_endpoint
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57694
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326477
milestone: "13.11"
type: development
group: group::continuous integration
default_enabled: true
...@@ -490,6 +490,36 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -490,6 +490,36 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
{ 'id' => job.id, 'name' => job.name, 'token' => job.token }, { 'id' => job.id, 'name' => job.name, 'token' => job.token },
{ 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token })
end end
describe 'preloading job_artifacts_archive' do
context 'when the feature flag is disabled' do
before do
stub_feature_flags(preload_associations_jobs_request_api_endpoint: false)
end
it 'queries the ci_job_artifacts table multiple times' do
expect { request_job }.to exceed_all_query_limit(1).for_model(::Ci::JobArtifact)
end
it 'queries the ci_builds table more than five times' do
expect { request_job }.to exceed_all_query_limit(5).for_model(::Ci::Build)
end
end
context 'when the feature flag is enabled' do
before do
stub_feature_flags(preload_associations_jobs_request_api_endpoint: true)
end
it 'queries the ci_job_artifacts table once only' do
expect { request_job }.not_to exceed_all_query_limit(1).for_model(::Ci::JobArtifact)
end
it 'queries the ci_builds table five times' do
expect { request_job }.not_to exceed_all_query_limit(5).for_model(::Ci::Build)
end
end
end
end end
context 'when pipeline have jobs with artifacts' do context 'when pipeline have jobs with artifacts' do
......
...@@ -20,6 +20,11 @@ module ExceedQueryLimitHelpers ...@@ -20,6 +20,11 @@ module ExceedQueryLimitHelpers
self self
end end
def for_model(model)
table = model.table_name if model < ActiveRecord::Base
for_query(/(FROM|UPDATE|INSERT INTO|DELETE FROM)\s+"#{table}"/)
end
def show_common_queries def show_common_queries
@show_common_queries = true @show_common_queries = true
self self
......
...@@ -225,6 +225,16 @@ RSpec.describe ExceedQueryLimitHelpers do ...@@ -225,6 +225,16 @@ RSpec.describe ExceedQueryLimitHelpers do
expect(test_matcher.actual_count).to eq(2) expect(test_matcher.actual_count).to eq(2)
end end
it 'can filter specific models' do
test_matcher = TestMatcher.new.for_model(TestQueries)
test_matcher.verify_count do
TestQueries.first
TestQueries.connection.execute('select 1')
end
expect(test_matcher.actual_count).to eq(1)
end
it 'can ignore specific queries' do it 'can ignore specific queries' do
test_matcher = TestMatcher.new.ignoring(/foobar/) test_matcher = TestMatcher.new.ignoring(/foobar/)
test_matcher.verify_count do test_matcher.verify_count do
......
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