Skip to content

Replace Ci::Runner with linear scopes

What does this MR do and why?

In this MR, we're switching the behavior of two scopes in the Ci::Runner model to user the linear version: belonging_to_group and belonging_to_parent_group_of_project. The new behavior is behind the linear_runner_ancestor_scopes feature flag.

Database queries

belonging_to_group

The original query was:

SELECT ci_runners.*
FROM ci_runners
INNER JOIN ci_runner_namespaces ON ci_runner_namespaces.runner_id = ci_runners.id
WHERE ci_runner_namespaces.namespace_id IN
    (WITH RECURSIVE base_and_ancestors AS (
                                             (SELECT namespaces.*
                                              FROM namespaces
                                              WHERE namespaces.type = 'Group'
                                                AND namespaces.id = 22)
                                           UNION
                                             (SELECT namespaces.*
                                              FROM namespaces,
                                                   base_and_ancestors
                                              WHERE namespaces.type = 'Group'
                                                AND namespaces.id = base_and_ancestors.parent_id)) SELECT namespaces.id
     FROM base_and_ancestors AS namespaces)

This is the query plan and the times for a group with 21 parents are (and warm caches):

Time: 2.523 ms
  - planning: 1.702 ms
  - execution: 0.821 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

The new query is:

SELECT ci_runners.*
FROM ci_runners
INNER JOIN ci_runner_namespaces ON ci_runner_namespaces.runner_id = ci_runners.id
WHERE ci_runner_namespaces.namespace_id IN
    (SELECT namespaces.id
     FROM
       (SELECT namespaces.*
        FROM namespaces
        WHERE namespaces.id IN
            (SELECT unnest(traversal_ids)
             FROM namespaces
             WHERE namespaces.id = 2)) namespaces)

This is the query plan and the times for a group with 21 parents are (and warm caches):

Time: 1.532 ms
  - planning: 1.230 ms
  - execution: 0.302 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

belonging_to_parent_group_of_project

The original query was:

SELECT ci_runners.*
FROM ci_runners
INNER JOIN ci_runner_namespaces ON ci_runner_namespaces.runner_id = ci_runners.id
INNER JOIN namespaces ON namespaces.id = ci_runner_namespaces.namespace_id
AND namespaces.type = 'Group'
WHERE namespaces.id IN
    (WITH RECURSIVE base_and_ancestors AS (
                                             (SELECT namespaces.*
                                              FROM namespaces
                                              INNER JOIN projects ON projects.namespace_id = namespaces.id
                                              WHERE namespaces.type = 'Group'
                                                AND projects.id = 1)
                                           UNION
                                             (SELECT namespaces.*
                                              FROM namespaces,
                                                   base_and_ancestors
                                              WHERE namespaces.type = 'Group'
                                                AND namespaces.id = base_and_ancestors.parent_id)) SELECT id
     FROM base_and_ancestors AS namespaces)

This is the query plan and the times for a project whose group with 21 parents are (and warm caches):

Time: 4.145 ms
  - planning: 3.305 ms
  - execution: 0.840 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

The new query is:

SELECT ci_runners.*
FROM ci_runners
INNER JOIN ci_runner_namespaces ON ci_runner_namespaces.runner_id = ci_runners.id
INNER JOIN namespaces ON namespaces.id = ci_runner_namespaces.namespace_id
AND namespaces.type = 'Group'
WHERE namespaces.id IN
    (SELECT namespaces.id
     FROM
       (SELECT namespaces.*
        FROM namespaces
        WHERE namespaces.id IN
            (SELECT unnest(traversal_ids)
             FROM namespaces
             INNER JOIN projects ON projects.namespace_id = namespaces.id
             WHERE projects.id = 1)) namespaces)

This is the query plan and the times for a project whose group with 21 parents are (and warm caches):

Time: 3.392 ms
  - planning: 2.862 ms
  - execution: 0.530 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

How to set up and validate locally

Example below:

  1. Enable the feature flag for linear scopes
    Feature.enable(:use_traversal_ids)
  2. Enable the feature flag for linear ancestors scopes
    Feature.enable(:use_traversal_ids_for_ancestor_scopes)
  3. Enable the feature flag for Ci::Runner linear ancestors scopes
    Feature.enable(:linear_runner_ancestor_scopes)
  4. Execute the following in the Rails console with the proper id depending on the scope testing:
Ci::Runner.belonging_to_group(22, include_ancestors: true)
Ci::Runner.belonging_to_parent_group_of_project(1)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Quality

  • Quality checklist confirmed
  1. I have self-reviewed this MR per code review guidelines.
  2. For the code that that this change impacts, I believe that the automated tests (Testing Guide) validate functionality that is highly important to users (including consideration of all test levels). If the existing automated tests do not cover this functionality, I have added the necessary additional tests or I have added an issue to describe the automation testing gap and linked it to this MR.
  3. I have considered the technical aspects of the impact of this change on both gitlab.com hosted customers and self-hosted customers.
  4. I have considered the impact of this change on the front-end, back-end, and database portions of the system where appropriate and applied frontend, backend and database labels accordingly.
  5. I have tested this MR in all supported browsers, or determiend that this testing is not needed.
  6. I have confirmed that this change is backwards compatible across updates, or I have decided that this does not apply.
  7. I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
  8. If I am introducing a new expectation for existing data, I have confirmed that existing data meets this expectation or I have made this expectation optional rather than required.

Performance, reliability, and availability

  • Performance, reliability, and availability checklist confirmed
  1. I am confident that this MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
  2. I have added information for database reviewers in the MR description, or I have decided that it is unnecessary. (Does this MR have database-related changes?)
  3. I have considered the availability and reliability risks of this change. I have also considered the scalability risk based on future predicted growth
  4. I have considered the performance, reliability and availability impacts of this change on large customers who may have significantly more data than the average customer.

Security

  • Security checklist confirmed
  1. I have confirmed that if this MR contains changes to processing or storing of credentials or tokens, authorization, and authentication methods, or other items described in the security review guidelines, I have added the label security and I have @-mentioned @gitlab-com/gl-security/appsec.

Deployment

  • Deployment checklist confirmed
  1. I have considered using a feature flag for this change because the change may be high risk. If I decided to use a feature flag, I plan to test the change in staging before I test it in production, and I have considered rolling it out to a subset of production customers before doing rolling it out to all customers. When to use a feature flag
  2. I have informed the Infrastructure department of a default setting or new setting change per definition of done, or decided that this is not needed.

Related to #339186 (closed)

Edited by Francisco Javier López

Merge request reports

Loading