Draft: Migrations to fix integer type differences
What does this MR do?
Related issue #271160
As part of the related issue, a script was used to capture all the schema differences between the expected (structure.sql
) and actual (GitLab.com production database). A couple tables were defined with variations in integer types, being either int
or bigint
.
Since we can't reason about the status of self-managed installations and narrow the range the column can hold, it's best to convert both columns to be bigint
to synchronize their definitions.
For clusters_applications_crossplane
, we can do this inline as the table is only a few hundred rows, and tens of KB in size.
We can't take the same approach for merge_request_assignees
, as it's several GB in size and around 20M rows on GitLab.com,
and increasing the size of the column from would result in a table rewrite. It's also not possible to use current migration helpers that support changing column types via background migration, as they don't define the newly-created column as NOT NULL
, which we need to eventually promote it to the primary key.
Changing migration helpers can cause breaking changes for historic migrations, so we'd prefer to avoid that for a one-off. Originally the plan was to copy the change_column_type_using_background_migration
migration helper logic into the migration and make a few changes to support the needs, but that's not possible either. After the new temporary column is added and populated by the background migration, we need to concurrently create a unique index on the column, so that it can be promoted to primary key during the swap. However, the swap is currently done in a follow-up background migration job, where we wouldn't want to create an index (because we'd be outside the scope of normal migrations).
As a result, the proposed change would be:
Migration 1 (this MR, release 13.5)
- add the temporary column (as not null, with default 0)
- install the trigger to populate the temporary column from
id
- queue the background migration to backfill the temporary column
Migration 2 (release 13.6)
- cleanup missed data from the background migration
- create a unique index concurrently on the temporary column
- drop the trigger to populate the temporary column
- move the sequence from
id
to the temporary column - drop the
id
column - rename the temporary column to
id
- promote the new
id
to primary key
With steps 3-7 being completed transactionally.
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. -
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