Merge branch 'remove-mr-diff-serialised-columns' into 'master'

Remove serialised diff and commit columns

Closes #39533

See merge request gitlab-org/gitlab-ce!15582

6 files changed, 94 insertions, 5 deletions

diff --git a/db/migrate/20171121135738_clean_up_from_merge_request_diffs_and_commits.rb b/db/migrate/20171121135738_clean_up_from_merge_request_diffs_and_commits.rb
new file mode 100644
index 00000000000..30cf08b29fc
--- /dev/null
+++ b/db/migrate/20171121135738_clean_up_from_merge_request_diffs_and_commits.rb
@@ -0,0 +1,36 @@
+class CleanUpFromMergeRequestDiffsAndCommits < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  class MergeRequestDiff < ActiveRecord::Base
+    self.table_name = 'merge_request_diffs'
+
+    include ::EachBatch
+  end
+
+  disable_ddl_transaction!
+
+  def up
+    Gitlab::BackgroundMigration.steal('DeserializeMergeRequestDiffsAndCommits')
+
+    # The literal '--- []\n' value is created by the import process and treated
+    # as null by the application, so we can ignore those - even if we were
+    # migrating, it wouldn't create any rows.
+    literal_prefix = Gitlab::Database.postgresql? ? 'E' : ''
+    non_empty = "
+      (st_commits IS NOT NULL AND st_commits != #{literal_prefix}'--- []\n')
+      OR
+      (st_diffs IS NOT NULL AND st_diffs != #{literal_prefix}'--- []\n')
+    ".squish
+
+    MergeRequestDiff.where(non_empty).each_batch(of: 500) do |relation, index|
+      range = relation.pluck('MIN(id)', 'MAX(id)').first
+
+      Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits.new.perform(*range)
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/migrate/limits_to_mysql.rb b/db/migrate/limits_to_mysql.rb
index 5cd9f3198e3..8f8d8f27410 100644
--- a/db/migrate/limits_to_mysql.rb
+++ b/db/migrate/limits_to_mysql.rb
@@ -3,8 +3,19 @@ class LimitsToMysql < ActiveRecord::Migration
   def up
     return unless ActiveRecord::Base.configurations[Rails.env]['adapter'] =~ /^mysql/
 
-    change_column :merge_request_diffs, :st_commits, :text, limit: 2147483647
-    change_column :merge_request_diffs, :st_diffs, :text, limit: 2147483647
+    # These columns were removed in 10.3, but this is called from two places:
+    # 1. A migration run after they were added, but before they were removed.
+    # 2. A rake task which can be run at any time.
+    #
+    # Because of item 2, we need these checks.
+    if column_exists?(:merge_request_diffs, :st_commits)
+      change_column :merge_request_diffs, :st_commits, :text, limit: 2147483647
+    end
+
+    if column_exists?(:merge_request_diffs, :st_diffs)
+      change_column :merge_request_diffs, :st_diffs, :text, limit: 2147483647
+    end
+
     change_column :snippets, :content, :text, limit: 2147483647
     change_column :notes, :st_diff, :text, limit: 2147483647
   end
diff --git a/db/post_migrate/20171121160421_remove_merge_request_diff_st_commits_and_st_diffs.rb b/db/post_migrate/20171121160421_remove_merge_request_diff_st_commits_and_st_diffs.rb
new file mode 100644
index 00000000000..3a7b2a7fac0
--- /dev/null
+++ b/db/post_migrate/20171121160421_remove_merge_request_diff_st_commits_and_st_diffs.rb
@@ -0,0 +1,10 @@
+class RemoveMergeRequestDiffStCommitsAndStDiffs < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    remove_column :merge_request_diffs, :st_commits, :text
+    remove_column :merge_request_diffs, :st_diffs, :text
+  end
+end
diff --git a/db/post_migrate/20171124095655_add_index_on_merge_request_diffs_merge_request_id_and_id.rb b/db/post_migrate/20171124095655_add_index_on_merge_request_diffs_merge_request_id_and_id.rb
new file mode 100644
index 00000000000..698df712c11
--- /dev/null
+++ b/db/post_migrate/20171124095655_add_index_on_merge_request_diffs_merge_request_id_and_id.rb
@@ -0,0 +1,17 @@
+class AddIndexOnMergeRequestDiffsMergeRequestIdAndId < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:merge_request_diffs, [:merge_request_id, :id])
+  end
+
+  def down
+    if index_exists?(:merge_request_diffs, [:merge_request_id, :id])
+      remove_concurrent_index(:merge_request_diffs, [:merge_request_id, :id])
+    end
+  end
+end
diff --git a/db/post_migrate/20171124100152_remove_index_on_merge_request_diffs_merge_request_diff_id.rb b/db/post_migrate/20171124100152_remove_index_on_merge_request_diffs_merge_request_diff_id.rb
new file mode 100644
index 00000000000..038e4807000
--- /dev/null
+++ b/db/post_migrate/20171124100152_remove_index_on_merge_request_diffs_merge_request_diff_id.rb
@@ -0,0 +1,17 @@
+class RemoveIndexOnMergeRequestDiffsMergeRequestDiffId < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    if index_exists?(:merge_request_diffs, :merge_request_id)
+      remove_concurrent_index(:merge_request_diffs, :merge_request_id)
+    end
+  end
+
+  def down
+    add_concurrent_index(:merge_request_diffs, :merge_request_id)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 69307f2ca2b..effb2604af2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1014,8 +1014,6 @@ ActiveRecord::Schema.define(version: 20171124150326) do
 
   create_table "merge_request_diffs", force: :cascade do |t|
     t.string "state"
-    t.text "st_commits"
-    t.text "st_diffs"
     t.integer "merge_request_id", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
@@ -1025,7 +1023,7 @@ ActiveRecord::Schema.define(version: 20171124150326) do
     t.string "start_commit_sha"
   end
 
-  add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", using: :btree
+  add_index "merge_request_diffs", ["merge_request_id", "id"], name: "index_merge_request_diffs_on_merge_request_id_and_id", using: :btree
 
   create_table "merge_request_metrics", force: :cascade do |t|
     t.integer "merge_request_id", null: false