Add preloading to fix N+1 queries in merge requests search [RUN ALL RSPEC] (!57284) · Merge requests · GitLab.org / GitLab

Terri Chu requested to merge fix-n+1-for-merge-requests-scope into master Mar 23, 2021

What does this MR do?

Bullet detected multiple N+1 query optimizations. Fixed by adding additional preloading to the web response. After fixing the initial 4 detected by bullet, it found a few more so those were also added.

I realized that the current N+1 specs did not catch these issues because they all used the same project/namespace. I refactored the specs and tested on the master branch to confirm they failed there for merge requests and did not fail in this branch. As a result, I found that issues and milestones also suffer from N+1 queries and will open up follow up issues to address each of them.

`master`

 ~/Pr/g/gdk/gitlab  master *21 !1  be spring rspec ee/spec/features/search/elastic/global_search_spec.rb:70
Running via Spring preloader in process 9137
WARNING: `around(:context)` hooks are not supported and behave like `around(:example). Called from /Users/terrichu/Projects/gitlab/gdk/gitlab/spec/support/omniauth_strategy.rb:36:in `block in <main>'.
Run options: include {:focus=>true, :locations=>{"./ee/spec/features/search/elastic/global_search_spec.rb"=>[70]}}
F  HTML screenshot: /Users/terrichu/Projects/gitlab/gdk/gitlab/tmp/capybara/global_elastic_search_i_do_not_overload_the_database_searching_merge_requests_behaves_like_an_effici.html


Failures:

  1) Global elastic search I do not overload the database searching merge requests behaves like an efficient database result avoids N+1 database queries
     Failure/Error:

       Expected a maximum of 33 queries, got 38:

       SELECT "users".* FROM "users" WHERE "users"."id" = 1 ORDER BY "users"."id" ASC LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT 1 AS one FROM "identities" WHERE "identities"."user_id" = 1 AND (provider LIKE 'ldap%' AND extern_uid IS NOT NULL) LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "user_preferences".* FROM "user_preferences" WHERE "user_preferences"."user_id" = 1 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "projects"."id" FROM "projects" INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id" WHERE "project_authorizations"."user_id" = 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "projects"."id" FROM "projects" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE "projects"."id" = 1 AND ("project_features"."merge_requests_access_level" IS NULL OR "project_features"."merge_requests_access_level" IN (20,30) OR ("project_features"."merge_requests_access_level" = 10 AND EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)))) AND (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)) OR projects.visibility_level IN (10,20)) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."id" IN (5, 3, 6, 4, 1, 2) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "users".* FROM "users" WHERE "users"."id" IN (2, 8, 9, 10, 11, 12) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "projects".* FROM "projects" WHERE "projects"."id" IN (1, 2, 3, 4, 5, 6) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_type" = 'Project' AND "routes"."source_id" IN (1, 2, 3, 4, 5, 6) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" IN (1, 3, 4, 5, 6, 7) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 5 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 3 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 6 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 4 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 1 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 1 AND "namespaces"."type" = 'Group' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT MAX("project_authorizations"."access_level") AS maximum_access_level, "project_authorizations"."user_id" AS project_authorizations_user_id FROM "project_authorizations" WHERE "project_authorizations"."project_id" = 1 AND "project_authorizations"."user_id" = 1 GROUP BY "project_authorizations"."user_id" /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 2 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "licenses".* FROM "licenses" ORDER BY "licenses"."id" DESC LIMIT 100 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 6 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 4 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 7 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 5 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 1 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 3 AND "routes"."source_type" = 'Namespace' LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "appearances".* FROM "appearances" ORDER BY "appearances"."id" ASC LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "user_statuses".* FROM "user_statuses" WHERE "user_statuses"."user_id" = 1 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT COUNT(*) FROM "projects" INNER JOIN "namespaces" ON "projects"."namespace_id" = "namespaces"."id" WHERE "namespaces"."owner_id" = 1 AND "namespaces"."type" IS NULL /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT 1 AS one FROM "users" WHERE "users"."id" = 1 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "users".* FROM "users" WHERE "users"."id" = 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT COUNT(*) FROM "issues" INNER JOIN "projects" ON "projects"."id" = "issues"."project_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (
             issues.confidential IS NOT TRUE
             OR (issues.confidential = TRUE
               AND (issues.author_id = 1
                 OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = 1 AND issue_id = issues.id)
                 OR EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = issues.project_id) AND (project_authorizations.access_level >= 20))))) AND (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 10)) OR projects.visibility_level IN (10,20)) AND ("project_features"."issues_access_level" IS NULL OR "project_features"."issues_access_level" IN (20,30) OR ("project_features"."issues_access_level" = 10 AND EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 10)))) AND ("issues"."state_id" IN (1)) AND (EXISTS (SELECT true FROM "issue_assignees" WHERE "issue_assignees"."user_id" IN (1) AND issue_id = issues.id)) AND "projects"."archived" = FALSE /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT COUNT(*) FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)) OR projects.visibility_level IN (10,20)) AND ("project_features"."merge_requests_access_level" IS NULL OR "project_features"."merge_requests_access_level" IN (20,30) OR ("project_features"."merge_requests_access_level" = 10 AND EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)))) AND ("merge_requests"."state_id" IN (1)) AND (EXISTS (SELECT true FROM "merge_request_assignees" WHERE "merge_request_assignees"."user_id" IN (1) AND merge_request_id = merge_requests.id)) AND "projects"."archived" = FALSE /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "users".* FROM "users" WHERE "users"."id" = 1 LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT COUNT(*) FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)) OR projects.visibility_level IN (10,20)) AND ("project_features"."merge_requests_access_level" IS NULL OR "project_features"."merge_requests_access_level" IN (20,30) OR ("project_features"."merge_requests_access_level" = 10 AND EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id) AND (project_authorizations.access_level >= 20)))) AND ("merge_requests"."state_id" IN (1)) AND "projects"."archived" = FALSE AND EXISTS (SELECT true FROM "merge_request_reviewers" WHERE merge_request_id = merge_requests.id AND "merge_request_reviewers"."user_id" = 1) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT COUNT(*) FROM "todos" WHERE "todos"."user_id" = 1 AND ("todos"."state" IN ('pending')) /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT 1 AS one FROM "geo_nodes" LIMIT 1 /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "broadcast_messages".* FROM "broadcast_messages" WHERE (ends_at > '2021-03-24 18:18:43.098440') AND "broadcast_messages"."broadcast_type" = 1 ORDER BY "broadcast_messages"."id" ASC /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/

       SELECT "broadcast_messages".* FROM "broadcast_messages" WHERE (ends_at > '2021-03-24 18:18:43.100411') AND "broadcast_messages"."broadcast_type" = 2 ORDER BY "broadcast_messages"."id" ASC /*application:test,correlation_id:7e08bee3-1c0d-4227-be81-e875721e663e,endpoint_id:SearchController#show*/
     Shared Example Group: "an efficient database result" called from ./ee/spec/features/search/elastic/global_search_spec.rb:76
     # ./ee/spec/features/search/elastic/global_search_spec.rb:40:in `block (3 levels) in <main>'
     # ./spec/spec_helper.rb:365:in `block (3 levels) in <main>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:356:in `block (2 levels) in <main>'
     # ./spec/spec_helper.rb:348:in `block (3 levels) in <main>'
     # ./lib/gitlab/application_context.rb:31:in `with_raw_context'
     # ./spec/spec_helper.rb:348:in `block (2 levels) in <main>'
     # ./spec/support/sidekiq.rb:21:in `block (3 levels) in <main>'
     # ./spec/support/sidekiq.rb:8:in `gitlab_sidekiq_inline'
     # ./spec/support/sidekiq.rb:21:in `block (2 levels) in <main>'
     # ./ee/spec/support/elastic.rb:21:in `block (2 levels) in <main>'
     # -e:1:in `<main>'

Finished in 32.92 seconds (files took 6.45 seconds to load)
1 example, 1 failure

Failed examples:

rspec './ee/spec/features/search/elastic/global_search_spec.rb[1:1:3:1:1]' # Global elastic search I do not overload the database searching merge requests behaves like an efficient database result avoids N+1 database queries

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

📋 Does this MR need a changelog?
- I have included a changelog entry.
- I have not included a changelog entry because _____.
Documentation (if required)
Code review guidelines
Merge request performance guidelines
Style guides
Database guides
Separation of EE specific content

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
[-] Tested in all supported browsers
[-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

[-] Label as security and @ mention @gitlab-com/gl-security/appsec
[-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
[-] Security reports checked/validated by a reviewer from the AppSec team

Edited Mar 25, 2021 by Terri Chu

Add preloading to fix N+1 queries in merge requests search [RUN ALL RSPEC]