Remove one usage of legacy stages (!56987) · Merge requests · GitLab.org / GitLab

Miguel Rincon requested to merge 321788-remove-legacy-stages into master Mar 18, 2021

What does this MR do?

Remove one instance of legacy_stages in the Pipeline Details template as indicated by 71c4c43f.

This change is part of a larger effort to remove legacy stages. This MR specifically removes the app/models/ci/legacy_stage.rb class as explained in #238586 (closed).

Screenshots (strongly suggested)

This MR should not introduce any changes in the UI, we confirmed that jobs in the pipeline details stays the same. For example we wanted to confirm that the order of retried jobs does not change:

Local example URLs:

Before	After

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 this is just a refactor.
[-] Documentation (if required)
Code review guidelines
Merge request performance guidelines
Style guides
Database guides
[-] Separation of EE specific content

Queries added/changed

The before/after is difficult to compare because legacy stages were instantiated manually from CommitStatus entities and not from querying the ci_stages table. But this is what I get in the console:

p = Ci::Pipeline.last; nil
st = p.stages.with_latest_and_retried_statuses.load; nil

yields:

Ci::Stage Load (1.1ms)  SELECT "ci_stages".* FROM "ci_stages" WHERE "ci_stages"."pipeline_id" = $1 ORDER BY "ci_stages"."position" ASC /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["pipeline_id", 47]]
Project Load (0.8ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 1]]
Namespace Load (0.8ms)  SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 22]]
CommitStatus Load (2.1ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE ("ci_builds"."retried" = $1 OR "ci_builds"."retried" IS NULL) AND "ci_builds"."stage_id" IN ($2, $3, $4, $5) ORDER BY "ci_builds"."name" ASC /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["retried", false], ["stage_id", 185], ["stage_id", 186], ["stage_id", 187], ["stage_id", 188]]
Ci::Pipeline Load (0.6ms)  SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 47]]
Ci::Pipeline Load (0.6ms)  SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 47]]
Project Load (0.5ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 1]]
Namespace Load (0.6ms)  SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 22]]
Project Load (0.5ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 1]]
Namespace Load (0.5ms)  SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 22]]
CommitStatus Load (1.1ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."retried" = $1 AND "ci_builds"."stage_id" IN ($2, $3, $4, $5) ORDER BY "ci_builds"."name" ASC /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["retried", true], ["stage_id", 185], ["stage_id", 186], ["stage_id", 187], ["stage_id", 188]]
Ci::Pipeline Load (0.4ms)  SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 47]]
Project Load (0.5ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 1]]
Namespace Load (0.5ms)  SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 /*application:console,line:/data/cache/bundle-2.7.2/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:98:in `exec_query'*/  [["id", 22]]

There are still some duplicate queries because latest_statuses and retried_statuses are two separate entity trees. I'm not sure if there is an easy way to fix this without a bigger refactor.

The only query that was added was:

SELECT
    "ci_builds".*
FROM
    "ci_builds"
WHERE ("ci_builds"."retried" = FALSE
    OR "ci_builds"."retried" IS NULL)
AND "ci_builds"."stage_id" IN (185, 186, 187, 188)
ORDER BY
    "ci_builds"."name" ASC

https://console.postgres.ai/gitlab/gitlab-production-tunnel/sessions/2887/commands/9430

This query isn't optimally executed because it needs to filter rows returned from the index, but since we can expect the number of builds per stage to be limited, I think this should be fine.

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

Related to #321788

Edited Mar 31, 2021 by Matthias Käppler

Remove one usage of legacy stages