Fix Ci::Runner#projects and Ci::Runner#groups doing cross-joins
What does this MR do and why?
This MR does 3 things:
- Fixes cross database queries for the
Ci::Runner#projects
- Fixes cross database queries for the
Ci::Runner#groups
- Add missing specs to existing methods being used.
This feature is behind a feature flag : ci_runner_projects_disable_joins
Database review
Migration output
$ bin/rails db:migrate
== 20220120085655 AddCiRunnerProjectIndexToRunnerIdAndProjectId: migrating ====
-- transaction_open?()
-> 0.0000s
-- index_exists?(:ci_runner_projects, [:runner_id, :project_id], {:name=>"index_ci_runner_projects_on_runner_id_and_project_id", :algorithm=>:concurrently})
-> 0.0049s
-- execute("SET statement_timeout TO 0")
-> 0.0007s
-- add_index(:ci_runner_projects, [:runner_id, :project_id], {:name=>"index_ci_runner_projects_on_runner_id_and_project_id", :algorithm=>:concurrently})
-> 0.0045s
-- execute("RESET statement_timeout")
-> 0.0007s
-- transaction_open?()
-> 0.0000s
-- indexes(:ci_runner_projects)
-> 0.0019s
-- remove_index(:ci_runner_projects, {:algorithm=>:concurrently, :name=>"index_ci_runner_projects_on_runner_id"})
-> 0.0034s
== 20220120085655 AddCiRunnerProjectIndexToRunnerIdAndProjectId: migrated (0.0202s)
$ bin/rails db:rollback
== 20220120085655 AddCiRunnerProjectIndexToRunnerIdAndProjectId: reverting ====
-- transaction_open?()
-> 0.0000s
-- index_exists?(:ci_runner_projects, :runner_id, {:name=>"index_ci_runner_projects_on_runner_id", :algorithm=>:concurrently})
-> 0.0024s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- add_index(:ci_runner_projects, :runner_id, {:name=>"index_ci_runner_projects_on_runner_id", :algorithm=>:concurrently})
-> 0.0039s
-- execute("RESET statement_timeout")
-> 0.0005s
-- transaction_open?()
-> 0.0000s
-- indexes(:ci_runner_projects)
-> 0.0016s
-- remove_index(:ci_runner_projects, {:algorithm=>:concurrently, :name=>"index_ci_runner_projects_on_runner_id_and_project_id"})
-> 0.0030s
== 20220120085655 AddCiRunnerProjectIndexToRunnerIdAndProjectId: reverted (0.0150s)
Query changed
Old SQL Query
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_user_id",
"projects"."shared_runners_enabled",
"projects"."runners_token",
"projects"."build_coverage_regex",
"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"."emails_disabled",
"projects"."max_pages_size",
"projects"."max_artifacts_size",
"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"
FROM
"projects"
INNER JOIN "ci_runner_projects" ON "projects"."id" = "ci_runner_projects"."project_id"
WHERE
"ci_runner_projects"."runner_id" = 8
New SQL - Query 1 | Query 2
SELECT
"ci_runner_projects"."project_id"
FROM
"ci_runner_projects"
WHERE
"ci_runner_projects"."runner_id" = 8
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_user_id",
"projects"."shared_runners_enabled",
"projects"."runners_token",
"projects"."build_coverage_regex",
"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"."emails_disabled",
"projects"."max_pages_size",
"projects"."max_artifacts_size",
"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"
FROM
"projects"
WHERE
"projects"."id" = 1
Why are we doing this?
As explained on our multi databases documentation it will not be possible to make a join query accessing our 2 databases.
To solve this problem we are leveraging disable_joins for Ci::Runner#projects
similar to what we did for Ci::Runner#groups.
disable_joins
works
How disable_joins
works by calling pluck
on the intermediate association, in this case it plucks the project_id
from ci_runner_projects
. We then use the plucked value to query for the Project
.
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 #338659 (closed)
Edited by Max Orefice