Merge branch '288005-finalize-push-events-payloads-bigint-conversion' into 'master'

Finalize conversion to bigint for push_event_payloads

See merge request gitlab-org/gitlab!64577

db/post_migrate/20210622041846_finalize_push_event_payloads_bigint_conversion.rb

+# frozen_string_literal: true
+
+class FinalizePushEventPayloadsBigintConversion < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  TABLE_NAME = 'push_event_payloads'
+  INDEX_NAME = 'index_push_event_payloads_on_event_id_convert_to_bigint'
+
+  def up
+    ensure_batched_background_migration_is_finished(
+      job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
+      table_name: TABLE_NAME,
+      column_name: 'event_id',
+      job_arguments: [["event_id"], ["event_id_convert_to_bigint"]]
+    )
+
+    swap_columns
+  end
+
+  def down
+    swap_columns
+  end
+
+  private
+
+  def swap_columns
+    add_concurrent_index TABLE_NAME, :event_id_convert_to_bigint, unique: true, name: INDEX_NAME
+
+    # Add a foreign key on `event_id_convert_to_bigint` before we swap the columns and drop the old FK (fk_36c74129da)
+    add_concurrent_foreign_key TABLE_NAME, :events, column: :event_id_convert_to_bigint, on_delete: :cascade
+
+    with_lock_retries(raise_on_exhaustion: true) do
+      # Swap column names
+      temp_name = 'event_id_tmp'
+      execute "ALTER TABLE #{quote_table_name(TABLE_NAME)} RENAME COLUMN #{quote_column_name(:event_id)} TO #{quote_column_name(temp_name)}"
+      execute "ALTER TABLE #{quote_table_name(TABLE_NAME)} RENAME COLUMN #{quote_column_name(:event_id_convert_to_bigint)} TO #{quote_column_name(:event_id)}"
+      execute "ALTER TABLE #{quote_table_name(TABLE_NAME)} RENAME COLUMN #{quote_column_name(temp_name)} TO #{quote_column_name(:event_id_convert_to_bigint)}"
+
+      # Swap defaults
+      change_column_default TABLE_NAME, :event_id, nil
+      change_column_default TABLE_NAME, :event_id_convert_to_bigint, 0
+
+      # Swap PK constraint
+      execute "ALTER TABLE #{TABLE_NAME} DROP CONSTRAINT push_event_payloads_pkey"
+      rename_index TABLE_NAME, INDEX_NAME, 'push_event_payloads_pkey'
+      execute "ALTER TABLE #{TABLE_NAME} ADD CONSTRAINT push_event_payloads_pkey PRIMARY KEY USING INDEX push_event_payloads_pkey"
+
+      # Drop original FK on the old int4 `event_id` (fk_36c74129da)
+      remove_foreign_key TABLE_NAME, name: concurrent_foreign_key_name(TABLE_NAME, :event_id)
+      # We swapped the columns but the FK for event_id is still using the old name for the event_id_convert_to_bigint column
+      # So we have to also swap the FK name now that we dropped the other one with the same
+      rename_constraint(
+        TABLE_NAME,
+        concurrent_foreign_key_name(TABLE_NAME, :event_id_convert_to_bigint),
+        concurrent_foreign_key_name(TABLE_NAME, :event_id)
+      )
+    end
+  end
+end

db/schema_migrations/20210622041846

+750cda544df323be99452d53aaf39933c8584fc6754ece45263e131884d980d0
\ No newline at end of file

db/structure.sql

@@ -17289,7 +17289,7 @@ ALTER SEQUENCE protected_tags_id_seq OWNED BY protected_tags.id;
 
 CREATE TABLE push_event_payloads (
     commit_count bigint NOT NULL,
-    event_id integer NOT NULL,
+    event_id_convert_to_bigint integer DEFAULT 0 NOT NULL,
     action smallint NOT NULL,
     ref_type smallint NOT NULL,
     commit_from bytea,
@@ -17297,7 +17297,7 @@ CREATE TABLE push_event_payloads (
     ref text,
     commit_title character varying(70),
     ref_count integer,
-    event_id_convert_to_bigint bigint DEFAULT 0 NOT NULL
+    event_id bigint NOT NULL
 );
 
 CREATE TABLE push_rules (

lib/gitlab/database/migration_helpers.rb

@@ -389,12 +389,14 @@ module Gitlab
       # * +logger+ - [Gitlab::JsonLogger]
       # * +env+ - [Hash] custom environment hash, see the example with `DISABLE_LOCK_RETRIES`
       def with_lock_retries(*args, **kwargs, &block)
+        raise_on_exhaustion = !!kwargs.delete(:raise_on_exhaustion)
         merged_args = {
           klass: self.class,
           logger: Gitlab::BackgroundMigration::Logger
         }.merge(kwargs)
 
-        Gitlab::Database::WithLockRetries.new(**merged_args).run(&block)
+        Gitlab::Database::WithLockRetries.new(**merged_args)
+          .run(raise_on_exhaustion: raise_on_exhaustion, &block)
       end
 
       def true_value
@@ -1614,6 +1616,13 @@ into similar problems in the future (e.g. when new tables are created).
         raise
       end
 
+      def rename_constraint(table_name, old_name, new_name)
+        execute <<~SQL
+          ALTER TABLE #{quote_table_name(table_name)}
+          RENAME CONSTRAINT #{quote_column_name(old_name)} TO #{quote_column_name(new_name)}
+        SQL
+      end
+
       private
 
       def validate_check_constraint_name!(constraint_name)

rubocop/cop/migration/with_lock_retries_disallowed_method.rb

@@ -22,6 +22,7 @@ module RuboCop
           remove_foreign_key_if_exists
           remove_foreign_key_without_error
           rename_index
+          rename_constraint
           table_exists?
           index_exists_by_name?
           foreign_key_exists?

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -2157,6 +2157,28 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
       buffer.rewind
       expect(buffer.read).to include("\"class\":\"#{model.class}\"")
     end
+
+    using RSpec::Parameterized::TableSyntax
+
+    where(raise_on_exhaustion: [true, false])
+
+    with_them do
+      it 'sets raise_on_exhaustion as requested' do
+        with_lock_retries = double
+        expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries)
+        expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: raise_on_exhaustion)
+
+        model.with_lock_retries(env: env, logger: in_memory_logger, raise_on_exhaustion: raise_on_exhaustion) { }
+      end
+    end
+
+    it 'does not raise on exhaustion by default' do
+      with_lock_retries = double
+      expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries)
+      expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false)
+
+      model.with_lock_retries(env: env, logger: in_memory_logger) { }
+    end
   end
 
   describe '#backfill_iids' do
@@ -2955,4 +2977,12 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
       end
     end
   end
+
+  describe '#rename_constraint' do
+    it "executes the statement to rename constraint" do
+      expect(model).to receive(:execute).with /ALTER TABLE "test_table"\nRENAME CONSTRAINT "fk_old_name" TO "fk_new_name"/
+
+      model.rename_constraint(:test_table, :fk_old_name, :fk_new_name)
+    end
+  end
 end