Skip to content

Fix cross join in MergeRequest#environments

Laura Montemayor requested to merge lm-fix-pipeline-dep-and-env-cross-joins into master

What does this MR do and why?

Fixes cross joins in MergeRequest#environments

A similar approach was used in EnvironmentStatus: !71894 (merged), where the data was fetched via the ci_builds_metadata table.

Old query:

AR

actual_head_pipeline.environments

SQL

SELECT DISTINCT "environments".* 
FROM "environments" 
INNER JOIN "deployments" 
ON "environments"."id" = "deployments"."environment_id" 
INNER JOIN "ci_builds" ON "deployments"."deployable_id" = "ci_builds"."id" WHERE "ci_builds"."type" = 'Ci::Build' 
AND "ci_builds"."commit_id" = 82 AND "deployments"."deployable_type" = 'CommitStatus'

New queries:

AR

build_for_actual_head_pipeline.joins(:metadata)
    .where.not('ci_builds_metadata.expanded_environment_name' => nil)
    .distinct('ci_builds_metadata.expanded_environment_name')
    .limit(100) 
    .pluck(:expanded_environment_name)

Environment.where(project: project, name: environments)

SQL

SELECT DISTINCT "expanded_environment_name" 
FROM "ci_builds" 
INNER JOIN "ci_builds_metadata" 
ON "ci_builds_metadata"."build_id" = "ci_builds"."id" 
WHERE "ci_builds"."type" = 'Ci::Build' 
AND (
       "ci_builds"."retried" = FALSE 
     OR "ci_builds"."retried" IS NULL) 
AND "ci_builds"."commit_id" = 82 
AND "ci_builds_metadata"."expanded_environment_name" IS NOT NULL LIMIT 100

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" = 20 LIMIT 1

SELECT "environments".* FROM "environments" WHERE "environments"."project_id" = 20 AND "environments"."name" IN ('Staging', 'Production')

Explain Plans

These are all with cold cache
Full execution plan for:

SELECT DISTINCT "environments".* 
FROM "environments" 
INNER JOIN "deployments" 
ON "environments"."id" = "deployments"."environment_id" 
INNER JOIN "ci_builds" ON "deployments"."deployable_id" = "ci_builds"."id" WHERE "ci_builds"."type" = 'Ci::Build' 
AND "ci_builds"."commit_id" = 82 AND "deployments"."deployable_type" = 'CommitStatus'

Summary:

Time: 56.708 ms  
  - planning: 26.510 ms  
  - execution: 30.198 ms  
    - I/O read: 29.915 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 15 (~120.00 KiB) from the buffer pool  
  - reads: 8 (~64.00 KiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

Full execution plan for:

SELECT DISTINCT "expanded_environment_name" 
FROM "ci_builds" 
INNER JOIN "ci_builds_metadata" 
ON "ci_builds_metadata"."build_id" = "ci_builds"."id" 
WHERE "ci_builds"."type" = 'Ci::Build' 
AND (
       "ci_builds"."retried" = FALSE 
     OR "ci_builds"."retried" IS NULL) 
AND "ci_builds"."commit_id" = 82 
AND "ci_builds_metadata"."expanded_environment_name" IS NOT NULL LIMIT 100

Summary:

Time: 32.324 ms
  - planning: 22.876 ms
  - execution: 9.448 ms
    - I/O read: 9.283 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 15 (~120.00 KiB) from the buffer pool
  - reads: 8 (~64.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Full execution plan for:

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" = 20 LIMIT 1

Summary:

Time: 7.376 ms  
  - planning: 0.538 ms  
  - execution: 6.838 ms  
    - I/O read: 6.626 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 1 (~8.00 KiB) from the buffer pool  
  - reads: 3 (~24.00 KiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

Full execution plan for:

SELECT "environments".* FROM "environments" WHERE "environments"."project_id" = 20 AND "environments"."name" IN ('Staging', 'Production')

Summary:

Time: 30.782 ms  
  - planning: 1.376 ms  
  - execution: 29.406 ms  
    - I/O read: 29.240 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 7 (~56.00 KiB) from the buffer pool  
  - reads: 4 (~32.00 KiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

How to set up and validate locally

> merge_request = Ci::MergeRequest.last 
> merge_request.environments

# It should return the environments and the query should not do a cross join with the ci tables.

#338658 (closed)

Edited by Dylan Griffith

Merge request reports