Skip to content

Draft: Remove ensure stage service (TO BE MERGED 17.2 or 17.3)

Laura Montemayor requested to merge remove-ensure-stage-service into master

This MR should be merged in 17.2 - feature flag will be enabled by default first (17.1)

This MR removes the EnsureStageService and the feature flag. Notes:

  • Feature flag has been enabled globally more than a year with no issues.
  • We have model level validation for ci_stage in CommitStatus, which is the parent class for GenericCommitStatus and Ci::Processable, which in turn is the parent class for Ci::Build and Ci::Bridge.
  • GenericCommitStatus, which is used in UpdatePagesService is already enforcing a stage, and it will always place the generic commit status at the end via the EXTERNAL_STAGE_IDX, which is set to 1_000_000.
  • The default stage test is set in Ci::Config::Entry::Stage

Initially, the intention before the removal of the service was that we should add a not null constraint to the stage_id column in the ci_builds table, but this has proven to be too costly and I don't think it's worth it. The issue is that we have a few records in production without a stage_id, and we needed to take care of them first before enforcing this validation at database level. The only way to take care of these records is via a batched background migration, but given that the table is partitioned AND it's got millions of records, this operation is waaaay too costly. As a rough plan, it would require adding an async index, doing a batched migration which will take forever, and then all the subsequent work of cleanup. We already have a model-level validation which will prevent new null values from being inserted. Convo I had about this here: https://gitlab.slack.com/archives/C3NBYFJ6N/p1716903893212369 (internal)

Edited by Laura Montemayor

Merge request reports

Loading