Skip to content

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.

  1. Execute this command on master

    gdk restart && bin/rspec spec/controllers/concerns/renders_commits_spec.rb:61
  2. Ensure that it fails

  3. Execute it after checking out to this MR branch

    gdk restart && bin/rspec spec/controllers/concerns/renders_commits_spec.rb:61
  4. 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.

Related to #344956 (closed)

Edited by Dmitry Gruzd

Merge request reports

Loading