Remove Select N+1 Query in Project Authorizations Refresh
What does this MR do and why?
This change removes an N+1 issue that occurs from fetching a project record for project authorization record that is inserted. This happens due to this line introduced in !156775 (merged).
https://gitlab.com/gitlab-org/gitlab/-/issues/479316#note_2184745991
Before:
モ bundle exec rspec spec/workers/authorized_projects_worker_spec.rb:31
Run options: include {:focus=>true, :locations=>{"./spec/workers/authorized_projects_worker_spec.rb"=>[31]}}
Test environment set up in 1.072537721 seconds
F
Failures:
1) AuthorizedProjectsWorker when running the worker prevents N+1 queries
Failure/Error:
expect { described_class.new.perform(user.id) }
.not_to exceed_query_limit(control).with_threshold(inserts + deletes + updates)
Expected a maximum of 4 (+4) queries, got 15:
Query Diff:
-----------
DELETE FROM "project_authorizations"...
-- (expected: 0, got: 1)
WHERE "project_authorizations"."user_id" = 176 AND "project_authorizations"."project_id" = 124
INSERT INTO "project_authorizations" ("user_id","project_id","access_level","is_unique")...
-- (expected: 0, got: 1)
VALUES (176, 122, 30, TRUE), (176, 123, 30, TRUE) ON CONFLICT ("user_id","project_id","access_level") DO NOTHING RETURNING "user_id","project_id","access_level"
SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_last_update_at", "projects"."mirror_last_successful_update_at", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."pull_mirror_branch_prefix", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden", "projects"."organization_id" FROM find_projects_by_id(124) AS projects WHERE ("projects"."id" IS NOT NULL) LIMIT 1...
-- (expected: 0, got: 1)
SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_last_update_at", "projects"."mirror_last_successful_update_at", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."pull_mirror_branch_prefix", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden", "projects"."organization_id" FROM find_projects_by_id(122) AS projects WHERE ("projects"."id" IS NOT NULL) LIMIT 1...
-- (expected: 0, got: 1)
SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_last_update_at", "projects"."mirror_last_successful_update_at", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."pull_mirror_branch_prefix", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden", "projects"."organization_id" FROM find_projects_by_id(123) AS projects WHERE ("projects"."id" IS NOT NULL) LIMIT 1...
-- (expected: 0, got: 1)
SAVEPOINT active_record_2...
-- (expected: 0, got: 1)
SELECT "user_details"."user_id", "user_details"."job_title", "user_details"."bio", "user_details"."webauthn_xid", "user_details"."provisioned_by_group_id", "user_details"."pronouns", "user_details"."pronunciation", "user_details"."registration_objective", "user_details"."phone", "user_details"."linkedin", "user_details"."twitter", "user_details"."skype", "user_details"."website_url", "user_details"."location", "user_details"."organization", "user_details"."password_last_changed_at", "user_details"."discord", "user_details"."enterprise_group_id", "user_details"."enterprise_group_associated_at", "user_details"."email_reset_offered_at", "user_details"."mastodon", "user_details"."project_authorizations_recalculated_at", "user_details"."onboarding_status", "user_details"."bluesky" FROM "user_details"...
-- (expected: 0, got: 1)
WHERE "user_details"."user_id" = 176 LIMIT 1
SELECT "namespaces"."id", "namespaces"."name", "namespaces"."path", "namespaces"."owner_id", "namespaces"."created_at", "namespaces"."updated_at", "namespaces"."type", "namespaces"."description", "namespaces"."avatar", "namespaces"."membership_lock", "namespaces"."share_with_group_lock", "namespaces"."visibility_level", "namespaces"."request_access_enabled", "namespaces"."ldap_sync_status", "namespaces"."ldap_sync_error", "namespaces"."ldap_sync_last_update_at", "namespaces"."ldap_sync_last_successful_update_at", "namespaces"."ldap_sync_last_sync_at", "namespaces"."description_html", "namespaces"."lfs_enabled", "namespaces"."parent_id", "namespaces"."shared_runners_minutes_limit", "namespaces"."repository_size_limit", "namespaces"."require_two_factor_authentication", "namespaces"."two_factor_grace_period", "namespaces"."cached_markdown_version", "namespaces"."project_creation_level", "namespaces"."runners_token", "namespaces"."file_template_project_id", "namespaces"."saml_discovery_token", "namespaces"."runners_token_encrypted", "namespaces"."custom_project_templates_group_id", "namespaces"."auto_devops_enabled", "namespaces"."extra_shared_runners_minutes_limit", "namespaces"."last_ci_minutes_notification_at", "namespaces"."last_ci_minutes_usage_notification_level", "namespaces"."subgroup_creation_level", "namespaces"."max_pages_size", "namespaces"."max_artifacts_size", "namespaces"."mentions_disabled", "namespaces"."default_branch_protection", "namespaces"."max_personal_access_token_lifetime", "namespaces"."push_rule_id", "namespaces"."shared_runners_enabled", "namespaces"."allow_descendants_override_disabled_shared_runners", "namespaces"."traversal_ids", "namespaces"."organization_id" FROM "namespaces"...
-- (expected: 0, got: 1)
WHERE "namespaces"."owner_id" = 176 AND "namespaces"."type" = 'User' LIMIT 1
SELECT 1 AS one FROM "users"...
-- (expected: 0, got: 1)
WHERE "users"."username" = 'user1' AND "users"."id" != 176 LIMIT 1
INSERT INTO "user_details" ("user_id", "project_authorizations_recalculated_at")...
-- (expected: 0, got: 1)
VALUES (176, '2024-10-30 19:22:28.875019') RETURNING "user_id"
RELEASE SAVEPOINT active_record_2...
-- (expected: 0, got: 1)
# ./spec/workers/authorized_projects_worker_spec.rb:53:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:483:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
# ./spec/spec_helper.rb:482:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:477:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:468:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:464:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:92:in `with_raw_context'
# ./spec/spec_helper.rb:464:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:277:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'
Finished in 6.08 seconds (files took 12.57 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/workers/authorized_projects_worker_spec.rb:31 # AuthorizedProjectsWorker when running the worker prevents N+1 queries
After:
モ bundle exec rspec spec/workers/authorized_projects_worker_spec.rb:31
Run options: include {:focus=>true, :locations=>{"./spec/workers/authorized_projects_worker_spec.rb"=>[31]}}
Test environment set up in 1.136426312 seconds
.
Finished in 5.86 seconds (files took 12.56 seconds to load)
1 example, 0 failures
References
Please include cross links to any resources that are relevant to this MR This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Edited by mo khan