Merge branch 'ensure-build-needs-bigint-backfill-self-hosted' into 'master'

Finish BigInt column swap for self-hosted installations

See merge request !123584



Merged-by: Laura Montemayor <lmontemayor@gitlab.com>
Approved-by: Etienne Baqué <ebaque@gitlab.com>
Reviewed-by: Krasimir Angelov <kangelov@gitlab.com>
Co-authored-by: Krasimir Angelov <kangelov@gitlab.com>

app/models/ci/build_need.rb

@@ -6,6 +6,8 @@ class BuildNeed < Ci::ApplicationRecord
     include BulkInsertSafe
     include IgnorableColumns
 
+    ignore_column :id_convert_to_bigint, remove_with: '16.4', remove_after: '2023-09-22'
+
     belongs_to :build, class_name: "Ci::Processable", foreign_key: :build_id, inverse_of: :needs
 
     partitionable scope: :build

db/post_migrate/20230613192703_ensure_ci_build_needs_big_int_backfill_is_finished_for_self_hosts.rb

+# frozen_string_literal: true
+
+class EnsureCiBuildNeedsBigIntBackfillIsFinishedForSelfHosts < Gitlab::Database::Migration[2.1]
+  include Gitlab::Database::MigrationHelpers::ConvertToBigint
+
+  disable_ddl_transaction!
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+  TABLE_NAME = 'ci_build_needs'
+
+  def up
+    ensure_batched_background_migration_is_finished(
+      job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
+      table_name: TABLE_NAME,
+      column_name: 'id',
+      job_arguments: [['id'], ['id_convert_to_bigint']]
+    )
+  end
+
+  def down
+    # no-op
+  end
+end

db/post_migrate/20230613192802_swap_ci_build_needs_to_big_int_for_self_hosts.rb

+# frozen_string_literal: true
+
+class SwapCiBuildNeedsToBigIntForSelfHosts < Gitlab::Database::Migration[2.1]
+  include Gitlab::Database::MigrationHelpers::ConvertToBigint
+
+  disable_ddl_transaction!
+
+  TABLE_NAME = 'ci_build_needs'
+
+  def up
+    return if should_skip?
+    return if temporary_column_already_dropped?
+    return if columns_already_swapped?
+
+    swap
+  end
+
+  def down
+    return if should_skip?
+    return if temporary_column_already_dropped?
+    return unless columns_already_swapped?
+
+    swap
+  end
+
+  private
+
+  def swap
+    add_concurrent_index TABLE_NAME, :id_convert_to_bigint, unique: true, name:
+      'index_ci_build_needs_on_id_convert_to_bigint'
+
+    with_lock_retries(raise_on_exhaustion: true) do
+      execute "LOCK TABLE #{TABLE_NAME} IN ACCESS EXCLUSIVE MODE"
+
+      execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN id TO id_tmp"
+      execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN id_convert_to_bigint TO id"
+      execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN id_tmp TO id_convert_to_bigint"
+
+      function_name = Gitlab::Database::UnidirectionalCopyTrigger.on_table(
+        TABLE_NAME, connection: Ci::ApplicationRecord.connection
+      ).name(
+        :id, :id_convert_to_bigint
+      )
+
+      execute "ALTER FUNCTION #{quote_table_name(function_name)} RESET ALL"
+
+      execute "ALTER SEQUENCE ci_build_needs_id_seq OWNED BY #{TABLE_NAME}.id"
+      change_column_default TABLE_NAME, :id, -> { "nextval('ci_build_needs_id_seq'::regclass)" }
+      change_column_default TABLE_NAME, :id_convert_to_bigint, 0
+
+      execute "ALTER TABLE #{TABLE_NAME} DROP CONSTRAINT ci_build_needs_pkey CASCADE"
+      rename_index TABLE_NAME, 'index_ci_build_needs_on_id_convert_to_bigint', 'ci_build_needs_pkey'
+      execute "ALTER TABLE #{TABLE_NAME} ADD CONSTRAINT ci_build_needs_pkey PRIMARY KEY USING INDEX ci_build_needs_pkey"
+    end
+  end
+
+  def should_skip?
+    com_or_dev_or_test_but_not_jh?
+  end
+
+  def columns_already_swapped?
+    table_columns = columns(TABLE_NAME)
+    column_id = table_columns.find { |c| c.name == 'id' }
+    column_id_convert_to_bigint = table_columns.find { |c| c.name == 'id_convert_to_bigint' }
+
+    column_id.sql_type == 'bigint' && column_id_convert_to_bigint.sql_type == 'integer'
+  end
+
+  def temporary_column_already_dropped?
+    table_columns = columns(TABLE_NAME)
+
+    !table_columns.find { |c| c.name == 'id_convert_to_bigint' }
+  end
+end

db/schema_migrations/20230613192703

+9a59cadd56d144591e828f8b64a7aa7887c156e86c75e1d57cc1e83fb8afbd01
\ No newline at end of file

db/schema_migrations/20230613192802

+3cf9c170bd70e70f0eef49c13317dcfbf79630d166417a61c6a2029ae40490c9
\ No newline at end of file

spec/migrations/20230613192703_ensure_ci_build_needs_big_int_backfill_is_finished_for_self_hosts_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe EnsureCiBuildNeedsBigIntBackfillIsFinishedForSelfHosts, migration: :gitlab_ci, feature_category: :continuous_integration do
+  describe '#up' do
+    let(:migration_arguments) do
+      {
+        job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
+        table_name: 'ci_build_needs',
+        column_name: 'id',
+        job_arguments: [['id'], ['id_convert_to_bigint']]
+      }
+    end
+
+    it 'ensures the migration is completed' do
+      expect_next_instance_of(described_class) do |instance|
+        expect(instance).to receive(:ensure_batched_background_migration_is_finished).with(migration_arguments)
+      end
+
+      migrate!
+    end
+  end
+end

spec/migrations/20230613192703_swap_ci_build_needs_to_big_int_for_self_hosts_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe SwapCiBuildNeedsToBigIntForSelfHosts, feature_category: :continuous_integration do
+  after do
+    connection = described_class.new.connection
+    connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+  end
+
+  describe '#up' do
+    context 'when on GitLab.com, dev, or test' do
+      before do
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE bigint')
+        connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+      end
+
+      it 'does not swap the columns' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        disable_migrations_output do
+          reversible_migration do |migration|
+            migration.before -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+
+            migration.after -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+          end
+        end
+      end
+    end
+
+    context 'when a self-hosted installation has already completed the swap' do
+      before do
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE bigint')
+        connection.execute('ALTER TABLE ci_build_needs ADD COLUMN IF NOT EXISTS id_convert_to_bigint integer')
+      end
+
+      it 'does not swap the columns' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        migrate!
+
+        expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+        expect(ci_build_needs.columns.find do |c|
+          c.name == 'id_convert_to_bigint'
+        end.sql_type).to eq('integer')
+      end
+    end
+
+    context 'when a self-hosted installation has the `id_convert_to_bigint` column already dropped' do
+      before do
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE bigint')
+        connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+      end
+
+      it 'does not swap the columns' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        disable_migrations_output do
+          reversible_migration do |migration|
+            migration.before -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+
+            migration.after -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+          end
+        end
+      end
+    end
+
+    context 'when an installation is self-hosted' do
+      before do
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE integer')
+        connection.execute('ALTER TABLE ci_build_needs ADD COLUMN IF NOT EXISTS id_convert_to_bigint bigint')
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id_convert_to_bigint TYPE bigint')
+        connection.execute('DROP INDEX IF EXISTS index_ci_build_needs_on_id_convert_to_bigint')
+        connection.execute('CREATE OR REPLACE FUNCTION trigger_3207b8d0d6f3() RETURNS trigger LANGUAGE plpgsql AS $$
+          BEGIN NEW."id_convert_to_bigint" := NEW."id"; RETURN NEW; END; $$;')
+      end
+
+      it 'swaps the columns' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        disable_migrations_output do
+          reversible_migration do |migration|
+            migration.before -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('integer')
+              expect(ci_build_needs.columns.find do |c|
+                       c.name == 'id_convert_to_bigint'
+                     end.sql_type).to eq('bigint')
+            }
+
+            migration.after -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find do |c|
+                       c.name == 'id_convert_to_bigint'
+                     end.sql_type).to eq('integer')
+            }
+          end
+        end
+      end
+    end
+  end
+end