Skip to content

Resolve "Avoid cross joins in environment model"

Bala Kumar requested to merge avoid-cross-joins-in-environment-model into master

What does this MR do and why?

It removes the cross join with ci_builds tables. Instead of preload(:deployable) we fetch the deployables and then do business logic on top of the fetched builds.

This join was originally done as a workaround for addressing via !47159 (merged)

This also fixes the long standing bug that with_deployable sometimes returns inexistent deployable object. This causes a huge noise on Sentry, so better to be properly handled.

To avoid the data integrity error, we also safely check for empty builds in some cases in the MR.

Screenshots or screen recordings

Old queries

> Deployment.active.builds

  Deployment Load (1.7ms)  SELECT "deployments".* FROM "deployments" INNER JOIN ci_builds ON ci_builds.id = deployments.deployable_id WHERE "deployments"."status" IN (0, 1)

  CommitStatus Load (0.7ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."id" IN (31, 32, 48, 49, 81, 134, 149, 251, 252, 253, 269, 270, 304, 338, 371, 406, 421, 422, 423, 438, 440, 610, 627, 643, 644, 660, 711, 712, 727, 808, 809)

New queries

> Deployment.active.builds

  (1.2ms)  SELECT "deployments"."deployable_id" FROM "deployments" WHERE "deployments"."status" IN (0, 1) AND "deployments"."deployable_id" IS NOT NULL LIMIT 1000

  Ci::Build Load (1.3ms)  SELECT "ci_builds"."status", "ci_builds"."finished_at", "ci_builds"."trace", "ci_builds"."created_at", "ci_builds"."updated_at", "ci_builds"."started_at", "ci_builds"."runner_id", "ci_builds"."coverage", "ci_builds"."commit_id", "ci_builds"."name", "ci_builds"."options", "ci_builds"."allow_failure", "ci_builds"."stage", "ci_builds"."trigger_request_id", "ci_builds"."stage_idx", "ci_builds"."tag", "ci_builds"."ref", "ci_builds"."user_id", "ci_builds"."type", "ci_builds"."target_url", "ci_builds"."description", "ci_builds"."project_id", "ci_builds"."erased_by_id", "ci_builds"."erased_at", "ci_builds"."artifacts_expire_at", "ci_builds"."environment", "ci_builds"."when", "ci_builds"."yaml_variables", "ci_builds"."queued_at", "ci_builds"."token", "ci_builds"."lock_version", "ci_builds"."coverage_regex", "ci_builds"."auto_canceled_by_id", "ci_builds"."retried", "ci_builds"."protected", "ci_builds"."failure_reason", "ci_builds"."scheduled_at", "ci_builds"."token_encrypted", "ci_builds"."upstream_pipeline_id", "ci_builds"."resource_group_id", "ci_builds"."waiting_for_resource_at", "ci_builds"."processed", "ci_builds"."scheduling_type", "ci_builds"."id", "ci_builds"."stage_id" FROM "ci_builds" WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."id" IN (270, 338, 253, 304, 269, 251, 252, 371, 423, 440, 423, 406, 438, 421, 438, 422, 712, 644, 627, 610, 727, 643, 711, 660, 49, 134, 32, 49, 48, 149, 81, 31, 808, 809)

How to set up and validate locally

> Deployment.active.builds

should give the builds for active deployments without cross joins with the CI tables.

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 #340188 (closed)

Edited by Shinya Maeda

Merge request reports