Optimize Deploy Key Presenter [RUN ALL RSPEC] [RUN AS-IF-FOSS] (!56305) · Merge requests · GitLab.org / GitLab

Shinya Maeda requested to merge optimize-deploy-key-presenter into master Mar 11, 2021

What does this MR do?

As discovered in the comment, DeployKeysPresenter#available_public_keys is hitting statement timeout and prevents customers from accessing project-level deploy key configuration page. This is because intensive usage on WHERE IN clause that executing the same subqueries over and over again.

You can see the full detail of the current query performance in #323964 (comment 526791073)

Time: 1.997 min
  - planning: 7.941 ms
  - execution: 1.997 min (estimated* for prod: 28.535...119.819 s)
    - I/O read: 4.991 min
    - I/O write: 241.326 ms

Shared buffers:
  - hits: 3923699 (~29.90 GiB) from the buffer pool
  - reads: 830355 (~6.30 GiB) from the OS file cache, including disk I/O
  - dirtied: 2884 (~22.50 MiB)
  - writes: 1207 (~9.40 MiB)

This MR mitigates the problem by fetching, and also filtering the target objects in ruby with memoization. This is rather considered as temporary/short-term solution, as ideally we should fetch all kinds of data via db query. However, this approach is practical enough because the expected memory consumption is same i.e. all key groups are fetched and presented to UI regardless of the filtering, hence it's allocated in memory already.

Query comparison

Accecing to http://local.gitlab.test:8181/root/dora-metrics/-/settings/repository, which renders the "Deploy Keys" section. It produces the following queries.

Before

------ `DeployKeysPresenter#available_keys`

SELECT
    "keys".*
FROM
    (
        (
            SELECT
                "keys".*
            FROM
                "keys"
            WHERE
                "keys"."type" = 'DeployKey'
                AND "keys"."id" IN (
                    SELECT
                        DISTINCT "deploy_key_id"
                    FROM
                        "keys"
                        INNER JOIN "deploy_keys_projects" ON "deploy_keys_projects"."deploy_key_id" = "keys"."id"
                    WHERE
                        "keys"."type" = 'DeployKey'
                        AND "deploy_keys_projects"."project_id" IN (
                            SELECT
                                "projects"."id"
                            FROM
                                "projects"
                                INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
                            WHERE
                                "project_authorizations"."user_id" = 1
                        )
                )
        )
        UNION
            (
                SELECT
                    "keys".*
                FROM
                    "keys"
                WHERE
                    "keys"."type" = 'DeployKey'
                    AND "keys"."public" = TRUE
            )
    ) keys
WHERE
    "keys"."type" = $ 1
    AND "keys"."id" NOT IN (
        SELECT
            "keys"."id"
        FROM
            "keys"
            INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
        WHERE
            "keys"."type" = $ 2
            AND "deploy_keys_projects"."project_id" = $ 3
    )
    
------ `DeployKeysPresenter#available_project_keys`

SELECT
    DISTINCT "keys".*
FROM
    "keys"
    INNER JOIN "deploy_keys_projects" ON "deploy_keys_projects"."deploy_key_id" = "keys"."id"
WHERE
    "keys"."type" = $ 1
    AND "deploy_keys_projects"."project_id" IN (
        SELECT
            "projects"."id"
        FROM
            "projects"
            INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
        WHERE
            "project_authorizations"."user_id" = $ 2
    )
    AND "keys"."id" NOT IN (
        SELECT
            "keys"."id"
        FROM
            "keys"
            INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
        WHERE
            "keys"."type" = $ 3
            AND "deploy_keys_projects"."project_id" = $ 4
    )
    
------ `DeployKeysPresenter#available_public_keys`

SELECT
    "keys".*
FROM
    "keys"
WHERE
    "keys"."type" = $ 1
    AND "keys"."public" = $ 2
    AND "keys"."id" NOT IN (
        SELECT
            "keys"."id"
        FROM
            "keys"
            INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
        WHERE
            "keys"."type" = $ 3
            AND "deploy_keys_projects"."project_id" = $ 4
    )
    AND "keys"."id" NOT IN (
        SELECT
            DISTINCT "keys"."id"
        FROM
            "keys"
            INNER JOIN "deploy_keys_projects" ON "deploy_keys_projects"."deploy_key_id" = "keys"."id"
        WHERE
            "keys"."type" = $ 5
            AND "deploy_keys_projects"."project_id" IN (
                SELECT
                    "projects"."id"
                FROM
                    "projects"
                    INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
                WHERE
                    "project_authorizations"."user_id" = $ 6
            )
            AND "keys"."id" NOT IN (
                SELECT
                    "keys"."id"
                FROM
                    "keys"
                    INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
                WHERE
                    "keys"."type" = $ 7
                    AND "deploy_keys_projects"."project_id" = $ 8
            )
    )
    
------ `DeployKeysPresenter#enabled_keys`

SELECT
    "keys".*
FROM
    "keys"
    INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
WHERE
    "keys"."type" = $ 1
    AND "deploy_keys_projects"."project_id" = $ 2

After

------ `DeployKeysPresenter#available_keys`

SELECT
    "keys".*
FROM
    (
        (
            SELECT
                "keys".*
            FROM
                "keys"
            WHERE
                "keys"."type" = 'DeployKey'
                AND "keys"."id" IN (
                    SELECT
                        DISTINCT "deploy_key_id"
                    FROM
                        "keys"
                        INNER JOIN "deploy_keys_projects" ON "deploy_keys_projects"."deploy_key_id" = "keys"."id"
                    WHERE
                        "keys"."type" = 'DeployKey'
                        AND "deploy_keys_projects"."project_id" IN (
                            SELECT
                                "projects"."id"
                            FROM
                                "projects"
                                INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
                            WHERE
                                "project_authorizations"."user_id" = 1
                        )
                )
        )
        UNION
            (
                SELECT
                    "keys".*
                FROM
                    "keys"
                WHERE
                    "keys"."type" = 'DeployKey'
                    AND "keys"."public" = TRUE
            )
    ) keys
WHERE
    "keys"."type" = $ 1
    
------ `DeployKeysPresenter#available_project_keys`

SELECT
    DISTINCT "keys".*
FROM
    "keys"
    INNER JOIN "deploy_keys_projects" ON "deploy_keys_projects"."deploy_key_id" = "keys"."id"
WHERE
    "keys"."type" = $ 1
    AND "deploy_keys_projects"."project_id" IN (
        SELECT
            "projects"."id"
        FROM
            "projects"
            INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
        WHERE
            "project_authorizations"."user_id" = $ 2
    )
    
------ `DeployKeysPresenter#available_public_keys`

SELECT
    "keys".*
FROM
    "keys"
WHERE
    "keys"."type" = $ 1
    AND "keys"."public" = $ 2

------ `DeployKeysPresenter#enabled_keys`

SELECT
    "keys".*
FROM
    "keys"
    INNER JOIN "deploy_keys_projects" ON "keys"."id" = "deploy_keys_projects"."deploy_key_id"
WHERE
    "keys"."type" = $ 1
    AND "deploy_keys_projects"."project_id" = $ 2

Please notice that the repeated "keys"."id" NOT IN condition is removed in the new version. This logic is translated to Ruby's Array operation.

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

📋 Does this MR need a changelog?
- I have included a changelog entry.
- I have not included a changelog entry because it's behind a feature flag.
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

Edited Mar 17, 2021 by Shinya Maeda

Optimize Deploy Key Presenter [RUN ALL RSPEC] [RUN AS-IF-FOSS]