Skip to content

Proof of Concept: Try to use Views to perform atomic column renames

Problem

Currently we effectively have no way to rename columns, thus this increases ~"technical debt" across codebase, as we cannot easily rename tables and columns, making the suboptimal structure in a place.

The current proposed methods makes use of table triggers, to continue writing table changes into two places, and requires to perform data migration.

This tries to avoid that, by using a VIEW which tries to resemble the structure after the change, but still allowing the previously running code to continue using an existing structure with old naming.

This is based on a unverified (yet) assumption that we can inject a VIEW transparently to the database without causing much interruption, and this is effectively no-cost to handle all logic via view for a limited period of time.

Using the same approach we can rename the whole tables.

What does this MR do?

Try to use Views to perform atomic column renames

This uses a combination of VIEWs with column aliasing to allow online column rename without performing a complex table rewrite via triggers.

This assumes that:

  • each step of the process we can do atomically, so it would be transparent to application, as it would create a short living lock on updating the structure
  • not needed with Iteration 2: we fix Rails to understand that it works on a View, so it would properly handle not-yet renamed column, but retaining all default values / primary keys / indexes supported (as they retain all values)
  • not needed with Iteration 2: we fix Rails to make it think that the new structure is in place, even though it might not be yet true, as it would be performing SELECT/INSERT/UPDATE on a views

What we do?

This splits the column rename process into two migrations:

  1. Regular migration that is executed before new code runs,
  2. Post deployment migration that is executed as a cleanup after a new code runs

Iteration 1

It uses CREATE VIEW AS ... SELECT FROM. This does not copy defaults and nullables, making us to fix Rails.

I1.1. Regular migration

-- transaction()
-- execute("ALTER TABLE ci_trigger_requests RENAME TO ci_trigger_requests_column_rename")
   -> 0.0012s
-- execute("CREATE VIEW ci_trigger_requests AS SELECT *, commit_id AS pipeline_id FROM ci_trigger_requests_column_rename")
   -> 0.0014s
   -> 0.0027s

I1.2. Post-deploy migration

-- transaction()
-- transaction()
-- execute("DROP VIEW ci_trigger_requests")
   -> 0.0014s
-- execute("ALTER TABLE ci_trigger_requests_column_rename RENAME TO ci_trigger_requests")
   -> 0.0004s
   -> 0.0019s
-- rename_column(:ci_trigger_requests, :commit_id, :pipeline_id)
   -> 0.0031s
   -> 0.0051s

Iteration 2

It now duplicates table structure. Adds a new column with defaults/nullables. This allows to completely remove all Rails "fixes", as from Rails perspective this behaves as "normal" table with the duplicated column.

I2.1. Regular migration

-- transaction()
-- columns(:ci_trigger_requests)
   (0.2ms)  BEGIN
   -> 0.0019s
-- execute("ALTER TABLE ci_trigger_requests RENAME TO ci_trigger_requests_column_rename")
   (0.2ms)  ALTER TABLE ci_trigger_requests RENAME TO ci_trigger_requests_column_rename
   -> 0.0003s
-- execute("CREATE TABLE ci_trigger_requests (LIKE ci_trigger_requests_column_rename INCLUDING DEFAULTS)")
   (5.4ms)  CREATE TABLE ci_trigger_requests (LIKE ci_trigger_requests_column_rename INCLUDING DEFAULTS)
   -> 0.0056s
-- add_column(:ci_trigger_requests, :pipeline_id, :integer, {:limit=>4, :precision=>nil, :scale=>nil})
   (0.4ms)  ALTER TABLE "ci_trigger_requests" ADD "pipeline_id" integer
   -> 0.0008s
-- execute("CREATE RULE \"_RETURN\" AS ON SELECT TO ci_trigger_requests DO INSTEAD SELECT *, commit_id AS pipeline_id FROM ci_trigger_requests_column_rename")
   (0.6ms)  CREATE RULE "_RETURN" AS ON SELECT TO ci_trigger_requests DO INSTEAD SELECT *, commit_id AS pipeline_id FROM ci_trigger_requests_column_rename
   -> 0.0008s
   (3.2ms)  COMMIT
   -> 0.0132s

I2.2. Post-deploy migration

-- transaction()
-- transaction()
-- execute("DROP VIEW ci_trigger_requests")
   -> 0.0014s
-- execute("ALTER TABLE ci_trigger_requests_column_rename RENAME TO ci_trigger_requests")
   -> 0.0004s
   -> 0.0019s
-- rename_column(:ci_trigger_requests, :commit_id, :pipeline_id)
   -> 0.0031s
   -> 0.0051s

I2.3. Structure

gitlabhq_development=# \d ci_trigger_requests;
                                         View "public.ci_trigger_requests"
   Column    |            Type             | Collation | Nullable |                     Default                     
-------------+-----------------------------+-----------+----------+-------------------------------------------------
 id          | integer                     |           | not null | nextval('ci_trigger_requests_id_seq'::regclass)
 trigger_id  | integer                     |           | not null | 
 variables   | text                        |           |          | 
 created_at  | timestamp without time zone |           |          | 
 updated_at  | timestamp without time zone |           |          | 
 commit_id   | integer                     |           |          | 
 pipeline_id | integer                     |           |          | 

Yay. We retain nullables and defaults, which allows us to completely fake Rails on a behavior.

Iteration 3

It reorders operation compared to Iteration 2.

We rename column to the destination name, and we duplicate it to make it "useable"

It now duplicates table structure. Adds a new column with defaults/nullables. This allows to completely remove all Rails "fixes", as from Rails perspective this behaves as "normal" table with the duplicated column. We create a view on top of table that allows us to serve (select/insert/update) using an old name.

As a cleanup we just quickly switch view with a table.

I3.1. Pre-migration

-- transaction()
-- columns(:ci_trigger_requests)
   (0.2ms)  BEGIN
   -> 0.0019s
-- rename_column(:ci_trigger_requests, :commit_id, :pipeline_id)
   (0.2ms)  ALTER TABLE "ci_trigger_requests" RENAME COLUMN "commit_id" TO "pipeline_id"
   (0.2ms)  ALTER INDEX "index_ci_trigger_requests_on_commit_id" RENAME TO "index_ci_trigger_requests_on_pipeline_id"
   -> 0.0025s
-- execute("ALTER TABLE ci_trigger_requests RENAME TO ci_trigger_requests_column_rename")
   (0.3ms)  ALTER TABLE ci_trigger_requests RENAME TO ci_trigger_requests_column_rename
   -> 0.0004s
-- execute("CREATE TABLE ci_trigger_requests (LIKE ci_trigger_requests_column_rename INCLUDING DEFAULTS)")
   (16.9ms)  CREATE TABLE ci_trigger_requests (LIKE ci_trigger_requests_column_rename INCLUDING DEFAULTS)
   -> 0.0172s
-- add_column(:ci_trigger_requests, :commit_id, :integer, {:limit=>4, :precision=>nil, :scale=>nil})
   (0.4ms)  ALTER TABLE "ci_trigger_requests" ADD "commit_id" integer
   -> 0.0007s
-- execute("CREATE RULE \"_RETURN\" AS ON SELECT TO ci_trigger_requests DO INSTEAD SELECT *, pipeline_id AS commit_id FROM ci_trigger_requests_column_rename")
   (6.3ms)  CREATE RULE "_RETURN" AS ON SELECT TO ci_trigger_requests DO INSTEAD SELECT *, pipeline_id AS commit_id FROM ci_trigger_requests_column_rename
   -> 0.0066s
   (2.7ms)  COMMIT
   -> 0.0325s

I3.2. Post-migration

-- transaction()
-- execute("DROP VIEW ci_trigger_requests")
   (0.2ms)  BEGIN
   (0.6ms)  DROP VIEW ci_trigger_requests
   -> 0.0012s
-- execute("ALTER TABLE ci_trigger_requests_column_rename RENAME TO ci_trigger_requests")
   (0.2ms)  ALTER TABLE ci_trigger_requests_column_rename RENAME TO ci_trigger_requests
   -> 0.0004s
   (13.6ms)  COMMIT
   -> 0.0155s

I3.3. Data structure

                                         View "public.ci_trigger_requests"
   Column    |            Type             | Collation | Nullable |                     Default                     
-------------+-----------------------------+-----------+----------+-------------------------------------------------
 id          | integer                     |           | not null | nextval('ci_trigger_requests_id_seq'::regclass)
 trigger_id  | integer                     |           | not null | 
 variables   | text                        |           |          | 
 created_at  | timestamp without time zone |           |          | 
 updated_at  | timestamp without time zone |           |          | 
 pipeline_id | integer                     |           |          | 
 commit_id   | integer                     |           |          | 

Yay. We retain nullables and defaults, which allows us to completely fake Rails on a behavior.

Un-tested

No longer valid as Iteration 3 fixes that. I expect that Rails 6 upsert will not work, as it reads table indexes to figure the ON CONFLICT clause. This is to validate.

With Iteration 3 we support fetching indexes from the seeding table, instead of view. Which will make the upsert code to work pretty OK in majority of the cases.

The only cases when they would not work, if: unique_by: uses a column that is being renamed. It is not allowed, as if schema gets reloaded on old version of application, it would result in inability to find relevant index.

BTW. The same problem is present with current helpers rename_column_concurrently, as they do not fix the indexes before all changes are made, so they are susceptible to the same problem as described above.

Does this MR meet the acceptance criteria?

Conformity

Edited by 🤖 GitLab Bot 🤖

Merge request reports