Skip to content

Fix Ci::Runner#projects and Ci::Runner#groups doing cross-joins

Max Orefice requested to merge mo-fix-ci-runner-projects-and-ci-runner-groups-doing-cross-joins into master

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.

How disable_joins works

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.

Related to #338659 (closed)

Edited by Max Orefice

Merge request reports