Fix spec failures due to PG::LockNotAvailable errors

We were seeing a high number of transient failures in the migration jobs
because `with_lock_retries` leaked non-zero, short `lock_timeout` values
(e.g. 100 ms) when used inside a Rails `change` method. If the
PostgreSQL autovacuum process happened to be running, it would lock the
table that it was vacuuming. During the migration rollback, if the DDL
operation needed a lock on the table, the short `lock_timeout` would
encounter the existing table lock and fail.

Even though `SET LOCAL` was used to ensure `lock_timeout` didn't leak
outside of the current transaction, the parent transaction would still
retain that value.

To avoid this issue, we should define separate `up` and `down` methods
so that we don't rely on the Rails magic to reverse a migration. This
ensures lock retries are used properly in both directions and prevents
`lock_timeout` from leaking during a migration rollback.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/207088

db/migrate/20200203025400_default_lock_version_to_zero_for_merge_requests.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForMergeRequests < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :merge_requests, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :merge_requests, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200203025602_default_lock_version_to_zero_for_issues.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForIssues < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :issues, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :issues, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200203025619_default_lock_version_to_zero_for_epics.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForEpics < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :epics, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :epics, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200203025744_default_lock_version_to_zero_for_ci_builds.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiBuilds < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :ci_builds, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :ci_builds, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200203025801_default_lock_version_to_zero_for_ci_stages.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiStages < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :ci_stages, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :ci_stages, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200203025821_default_lock_version_to_zero_for_ci_pipelines.rb

@@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiPipelines < ActiveRecord::Migration[6.0]
   # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       change_column_default :ci_pipelines, :lock_version, from: nil, to: 0
     end
   end
+
+  def down
+    with_lock_retries do
+      change_column_default :ci_pipelines, :lock_version, from: 0, to: nil
+    end
+  end
 end

db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb

@@ -5,9 +5,15 @@ class AddDefaultBranchProtectionToNamespaces < ActiveRecord::Migration[6.0]
 
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       add_column :namespaces, :default_branch_protection, :integer, limit: 2
     end
   end
+
+  def down
+    with_lock_retries do
+      remove_column :namespaces, :default_branch_protection
+    end
+  end
 end

db/migrate/20200222055543_add_confidential_to_note.rb

@@ -4,9 +4,15 @@ class AddConfidentialToNote < ActiveRecord::Migration[6.0]
 
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       add_column :notes, :confidential, :boolean
     end
   end
+
+  def down
+    with_lock_retries do
+      remove_column :notes, :confidential
+    end
+  end
 end

db/migrate/20200227165129_create_user_details.rb

@@ -5,7 +5,7 @@ class CreateUserDetails < ActiveRecord::Migration[6.0]
 
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       create_table :user_details, id: false do |t|
         t.references :user, index: false, foreign_key: { on_delete: :cascade }, null: false, primary_key: true
@@ -15,4 +15,10 @@ class CreateUserDetails < ActiveRecord::Migration[6.0]
 
     add_index :user_details, :user_id, unique: true
   end
+
+  def down
+    with_lock_retries do
+      drop_table :user_details
+    end
+  end
 end

db/migrate/20200304121828_add_ci_sources_project_pipeline_foreign_key.rb

@@ -5,9 +5,15 @@ class AddCiSourcesProjectPipelineForeignKey < ActiveRecord::Migration[6.0]
 
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       add_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
     end
   end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id
+    end
+  end
 end

db/migrate/20200304121844_add_ci_sources_project_source_project_foreign_key.rb

@@ -5,9 +5,15 @@ class AddCiSourcesProjectSourceProjectForeignKey < ActiveRecord::Migration[6.0]
 
   DOWNTIME = false
 
-  def change
+  def up
     with_lock_retries do
       add_foreign_key :ci_sources_projects, :projects, column: :source_project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
     end
   end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ci_sources_projects, :projects, column: :source_project_id
+    end
+  end
 end