Add migration to cleanup internal_ids.ab-49446-internal-ids-inconsistency

See https://gitlab.com/gitlab-org/gitlab-ce/issues/49446.

4 files changed, 189 insertions, 1 deletions

diff --git a/changelogs/unreleased/ab-49446-internal-ids-inconsistency.yml b/changelogs/unreleased/ab-49446-internal-ids-inconsistency.yml
new file mode 100644
index 00000000000..83c66043b0f
--- /dev/null
+++ b/changelogs/unreleased/ab-49446-internal-ids-inconsistency.yml
@@ -0,0 +1,5 @@
+---
+title: Add migration to cleanup internal_ids.
+merge_request: 20786
+author:
+type: fixed
diff --git a/db/post_migrate/20180723130817_delete_inconsistent_internal_id_records.rb b/db/post_migrate/20180723130817_delete_inconsistent_internal_id_records.rb
new file mode 100644
index 00000000000..ff654821348
--- /dev/null
+++ b/db/post_migrate/20180723130817_delete_inconsistent_internal_id_records.rb
@@ -0,0 +1,54 @@
+class DeleteInconsistentInternalIdRecords < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # This migration cleans up any inconsistent records in internal_ids.
+  #
+  # That is, it deletes records that track a `last_value` that is
+  # smaller than the maximum internal id (usually `iid`) found in
+  # the corresponding model records.
+
+  def up
+    disable_statement_timeout
+
+    delete_internal_id_records('issues', 'project_id')
+    delete_internal_id_records('merge_requests', 'project_id', 'target_project_id')
+    delete_internal_id_records('deployments', 'project_id')
+    delete_internal_id_records('milestones', 'project_id')
+    delete_internal_id_records('milestones', 'namespace_id', 'group_id')
+    delete_internal_id_records('ci_pipelines', 'project_id')
+  end
+
+  class InternalId < ActiveRecord::Base
+    self.table_name = 'internal_ids'
+    enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4, ci_pipelines: 5 }
+  end
+
+  private
+
+  def delete_internal_id_records(base_table, scope_column_name, base_scope_column_name = scope_column_name)
+    sql = <<~SQL
+       SELECT internal_ids.id FROM (
+         SELECT #{base_scope_column_name} AS #{scope_column_name}, max(iid) as maximum_iid from #{base_table} GROUP BY #{scope_column_name}
+       ) maxima JOIN internal_ids USING (#{scope_column_name})
+       WHERE internal_ids.usage=#{usage_for(base_table)} AND maxima.maximum_iid > internal_ids.last_value
+    SQL
+
+    InternalId.transaction do
+      ids = InternalId.where("id IN (#{sql})") # rubocop:disable GitlabSecurity/SqlInjection
+
+      ids.each do |id|
+        say "Deleting internal_id record for #{base_table}, project_id=#{id.project_id}, last_value=#{id.last_value}"
+      end
+
+      ids.destroy_all
+    end
+  end
+
+  def usage_for(base_table)
+    InternalId.usages[base_table] || raise("unknown base_table '#{base_table}'")
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 8ae0197d1b4..b76456576f3 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180722103201) do
+ActiveRecord::Schema.define(version: 20180723130817) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
new file mode 100644
index 00000000000..8235dcacbef
--- /dev/null
+++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
@@ -0,0 +1,129 @@
+# rubocop:disable RSpec/FactoriesInMigrationSpecs
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180723130817_delete_inconsistent_internal_id_records.rb')
+
+describe DeleteInconsistentInternalIdRecords, :migration do
+  let!(:project1) { create(:project) }
+  let!(:project2) { create(:project) }
+  let!(:project3) { create(:project) }
+
+  let(:internal_id_query) { ->(project) { InternalId.where(usage: InternalId.usages[scope.to_s.tableize], project: project) } }
+  let(:scope_column) { :project }
+  let(:model_columns) { {} }
+
+  def at_least_times(how_often, &block)
+    [how_often, rand(how_often + 5)].max.times(&block)
+  end
+
+  let(:create_models) do
+    -> do
+      at_least_times(3) { create(scope, scope_column => project1) }
+      at_least_times(3) { create(scope, scope_column => project2) }
+      at_least_times(3) { create(scope, scope_column => project3) }
+    end
+  end
+
+  shared_examples_for 'deleting inconsistent internal_id records' do
+    before do
+      create_models.call()
+
+      internal_id_query.call(project1).first.tap do |iid|
+        iid.last_value = iid.last_value - 2
+        # This is an inconsistent record
+        iid.save!
+      end
+
+      internal_id_query.call(project3).first.tap do |iid|
+        iid.last_value = iid.last_value + 2
+        # This is a consistent record
+        iid.save!
+      end
+    end
+
+    it "deletes inconsistent issues" do
+      expect { migrate! }.to change { internal_id_query.call(project1).size }.from(1).to(0)
+    end
+
+    it "retains consistent issues" do
+      expect { migrate! }.not_to change { internal_id_query.call(project2).size }
+    end
+
+    it "retains consistent records, especially those with a greater last_value" do
+      expect { migrate! }.not_to change { internal_id_query.call(project3).size }
+    end
+  end
+
+  context 'for issues' do
+    let(:scope) { :issue }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for merge_requests' do
+    let(:scope) { :merge_request }
+    let(:scope_column) { :target_project }
+
+    let(:create_models) do
+      -> do
+        at_least_times(3) { |i| create(scope, scope_column => project1, source_project: project1, source_branch: i.to_s) }
+        at_least_times(3) { |i| create(scope, scope_column => project2, source_project: project2, source_branch: i.to_s) }
+        at_least_times(3) { |i| create(scope, scope_column => project3, source_project: project3, source_branch: i.to_s) }
+      end
+    end
+
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for deployments' do
+    let(:scope) { :deployment }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for milestones (by project)' do
+    let(:scope) { :milestone }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for ci_pipelines' do
+    let(:scope) { :ci_pipeline }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for milestones (by group)' do
+    # milestones (by group) is a little different than all of the other models
+    let!(:group1) { create(:group) }
+    let!(:group2) { create(:group) }
+    let!(:group3) { create(:group) }
+
+    let(:internal_id_query) { ->(group) { InternalId.where(usage: InternalId.usages['milestones'], namespace: group) } }
+
+    before do
+      at_least_times(3) { create(:milestone, group: group1) }
+      at_least_times(3) { create(:milestone, group: group2) }
+      at_least_times(3) { create(:milestone, group: group3) }
+
+      internal_id_query.call(group1).first.tap do |iid|
+        iid.last_value = iid.last_value - 2
+        # This is an inconsistent record
+        iid.save!
+      end
+
+      internal_id_query.call(group3).first.tap do |iid|
+        iid.last_value = iid.last_value + 2
+        # This is a consistent record
+        iid.save!
+      end
+    end
+
+    it "deletes inconsistent issues" do
+      expect { migrate! }.to change { internal_id_query.call(group1).size }.from(1).to(0)
+    end
+
+    it "retains consistent issues" do
+      expect { migrate! }.not_to change { internal_id_query.call(group2).size }
+    end
+
+    it "retains consistent records, especially those with a greater last_value" do
+      expect { migrate! }.not_to change { internal_id_query.call(group3).size }
+    end
+  end
+end