Fix prepare_commits_for_rendering N+1 spec
What does this MR do and why?
This MR fixes #344956 (closed) (spec/controllers/concerns/renders_commits_spec.rb:61
) when it's executed for the first time. Subsequent runs pass because the Banzai::Filter::References::ReferenceCache
is populated. This MR ensures that this cache is always populated before we execute prepare_commits_for_rendering
against the entire array.
Failures:
1) RendersCommits.prepare_commits_for_rendering avoids N+1
Failure/Error:
expect do
subject.prepare_commits_for_rendering(merge_request.commits)
merge_request.commits.each(&:latest_pipeline)
end.not_to exceed_all_query_limit(control.count)
Expected a maximum of 5 queries, got 12:
# ........................................
# ........................................
# ........................................
# ./spec/controllers/concerns/renders_commits_spec.rb:67:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:416:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:407:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:403:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:31:in `with_raw_context'
# ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:207:in `block (2 levels) in <top (required)>'
# ./spec/support/database/prevent_cross_joins.rb:102:in `block (3 levels) in <top (required)>'
# ./spec/support/database/prevent_cross_joins.rb:56:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:102:in `block (2 levels) in <top (required)>'
Click to see extra queries
D, [2021-11-08T14:50:43.109890 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 11 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.110229 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.141571 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 10 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.141882 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.190876 #1826866] DEBUG -- : MergeRequest Load (0.3ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 9 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.191162 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.234905 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 6 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.235238 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.278617 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 5 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.278951 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.310215 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 4 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.310536 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.372556 #1826866] DEBUG -- : MergeRequest Load (0.4ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 3 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.372934 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
D, [2021-11-08T14:50:43.469862 #1826866] DEBUG -- : MergeRequest Load (0.3ms) SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 22 AND "merge_requests"."iid" = 2 /*application:test,correlation_id:93c8c95cfeebef9ebc96637aef15cd0c,db_config_name:main*/
D, [2021-11-08T14:50:43.470182 #1826866] DEBUG -- : ↳ lib/banzai/filter/references/reference_cache.rb:90:in `block in load_records_per_parent'
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
-
Execute this command on
master
gdk restart && bin/rspec spec/controllers/concerns/renders_commits_spec.rb:61
-
Ensure that it fails
-
Execute it after checking out to this MR branch
gdk restart && bin/rspec spec/controllers/concerns/renders_commits_spec.rb:61
-
Ensure that now it succeeds
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #344956 (closed)