Skip to content

Draft: Migrations to fix integer type differences

Patrick Bair requested to merge 211781-fix-different-integer-types into master

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)

  1. add the temporary column (as not null, with default 0)
  2. install the trigger to populate the temporary column from id
  3. queue the background migration to backfill the temporary column

Migration 2 (release 13.6)

  1. cleanup missed data from the background migration
  2. create a unique index concurrently on the temporary column
  3. drop the trigger to populate the temporary column
  4. move the sequence from id to the temporary column
  5. drop the id column
  6. rename the temporary column to id
  7. promote the new id to primary key

With steps 3-7 being completed transactionally.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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 by Patrick Bair

Merge request reports