Fix query timeout when finding auto-cancelable pipelines

What does this MR do?

Related to #273733 (closed)

In the attached issue we found that the query for auto-canceling pipelines was sometimes timing out since we changed the method same_family_pipeline_ids to use a recursive query to find out all pipelines in the same family including child of child pipelines. Prior to that we didn't have a recursive query because a parent pipeline could only trigger child pipelines. Today we allow child pipelines to trigger their child pipelines, hence the reason for the recursive query.

The approach taken to try to solve this is to break down the query from same_family_pipeline_ids into 2 queries:

1. find the root ancestor if self.child?, otherwise return self. The most common scenario would be that self is not a child pipeline.
2. find the descendants from the root ancestor. This query will always run.

Feature flag

This change is done behind a feature flag :ci_root_ancestor_for_pipeline_family that is going to be removed in #287812 (closed)

Query plans for Ci::Pipeline#same_family_pipeline_ids

With Feature Flag disabled (before):

WITH RECURSIVE "base_and_descendants" AS ((WITH RECURSIVE "base_and_ancestors" AS ((SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 208831043)
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines", "base_and_ancestors", "ci_sources_pipelines" WHERE "ci_sources_pipelines"."source_pipeline_id" = "ci_pipelines"."id" AND "ci_sources_pipelines"."pipeline_id" = "base_and_ancestors"."id" AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) SELECT "ci_pipelines".* FROM "base_and_ancestors" AS "ci_pipelines")
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines", "base_and_descendants", "ci_sources_pipelines" WHERE "ci_sources_pipelines"."pipeline_id" = "ci_pipelines"."id" AND "ci_sources_pipelines"."source_pipeline_id" = "base_and_descendants"."id" AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) SELECT "id" FROM "base_and_descendants" AS "ci_pipelines";

plan: https://explain.depesz.com/s/GM6I

Time: 31.811 ms
  - planning: 1.959 ms
  - execution: 29.852 ms
    - I/O read: 29.116 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 26 (~208.00 KiB) from the buffer pool
  - reads: 12 (~96.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

With Feature Flag enabled (after):

The first query will run only if the given pipeline is a child pipeline. In most of the cases it won't.

WITH RECURSIVE "base_and_ancestors" AS ((SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 208831043)
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines", "base_and_ancestors", "ci_sources_pipelines" WHERE "ci_sources_pipelines"."source_pipeline_id" = "ci_pipelines"."id" AND "ci_sources_pipelines"."pipeline_id" = "base_and_ancestors"."id" AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) SELECT "ci_pipelines".* FROM "base_and_ancestors" AS "ci_pipelines";

plan: https://explain.depesz.com/s/cRGP

Time: 1.371 ms
  - planning: 1.194 ms
  - execution: 0.177 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

The second query will run always.

WITH RECURSIVE "base_and_descendants" AS ((SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 208831026)
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines", "base_and_descendants", "ci_sources_pipelines" WHERE "ci_sources_pipelines"."pipeline_id" = "ci_pipelines"."id" AND "ci_sources_pipelines"."source_pipeline_id" = "base_and_descendants"."id" AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) SELECT "id" FROM "base_and_descendants" AS "ci_pipelines";

plan: https://explain.depesz.com/s/tenh

Time: 1.714 ms
  - planning: 1.256 ms
  - execution: 0.458 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

• Changelog entry
• Documentation (if required)
• Code review guidelines
• Merge request performance guidelines
• Style guides
• Database guides
• Separation of EE specific content

Availability and Testing

• Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
• Tested in all supported browsers
• Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

• Label as security and @ mention @gitlab-com/gl-security/appsec
• The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
• Security reports checked/validated by a reviewer from the AppSec team

changed milestone to %13.6

added bugperformance devopsverify gitlab.com grouppipeline execution priority1 severity1 typebug labels

marked this merge request as draft

added backend label

1 Message

CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, run the following: bin/changelog -m 46575 "Fix query timeout when finding auto-cancelable pipelines" If you want to create a changelog entry for GitLab EE, run the following instead: bin/changelog --ee -m 46575 "Fix query timeout when finding auto-cancelable pipelines" If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited, or the chosen person is unavailable.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.

Category	Reviewer	Maintainer
backend	Tiger Watson (@tigerwnz) (UTC+11, 11 hours ahead of @fabiopitino)	Mayra Cabrera (@mayra-cabrera) (UTC-6, 6 hours behind @fabiopitino)
database	Can Eldem (@caneldem) (UTC+0, same timezone as @fabiopitino)	Yannis Roussos (@iroussos) (UTC+2, 2 hours ahead of @fabiopitino)

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

mentioned in issue #273733 (closed)

added sectionops label

changed milestone to %13.7

added missed:13.6 label

added 3784 commits

• e033fde2...6ff76f6a - 3783 commits from branch master
• 413deda3 - Split same_family_pipeline_ids query to reduce complexity

Compare with previous version

marked this merge request as ready

changed the description

added 1 commit

• 0db6c704 - Split same_family_pipeline_ids query to reduce complexity

Compare with previous version

changed the description

added feature flag label

mentioned in issue #287812 (closed)

changed the description

mentioned in issue #287814 (closed)

added typemaintenance label

added typefeature label

@mbobin could you please review this, including database?

assigned to @mbobin

@mbobin This refactoring may improve also query plans in !48342 (diffs, comment 453318534)

added database databasereview pending labels

Thanks, @fabiopitino! It looks good! I have suggestions only for specs to reduce the number of database inserts.

added databasereviewed label and removed databasereview pending label

unassigned @mbobin

@iroussos could you do the database maintainer review here?

assigned to @iroussos

added 1 commit

• 2f665826 - Split same_family_pipeline_ids query to reduce complexity

Compare with previous version

resolved all threads

Thank you @fabiopitino, the update looks good on the database side

Back to you to merge or assign to a backend maintainer if you think that it is required.

approved this merge request

added databaseapproved label and removed databasereviewed label

unassigned @iroussos

@smcgivern could you please do the maintainer review?

assigned to @smcgivern

unassigned @smcgivern

added 1 commit

• 929ab2ad - Split same_family_pipeline_ids query to reduce complexity

Compare with previous version

assigned to @smcgivern

resolved all threads

approved this merge request

merged

mentioned in commit 3ea3f960

mentioned in issue gitlab-com/gl-infra/scalability#690

mentioned in merge request !48342 (merged)

added workflowstaging label

added workflowcanary label and removed workflowstaging label

added workflowproduction label and removed workflowcanary label

added releasedcandidate label

mentioned in merge request !49240 (merged)

added releasedpublished label and removed releasedcandidate label

removed typefeature label