Add migration to cleanup internal_ids.ab-49754-gh-importer-internal-ids

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

3 files changed, 171 insertions, 0 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..bfea57d79e0
--- /dev/null
+++ b/changelogs/unreleased/ab-49446-internal-ids-inconsistency.yml
@@ -0,0 +1,5 @@
+---
+title: Add migration to cleanup internal_ids inconsistency.
+merge_request: 20926
+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..3b9b95ec9ca
--- /dev/null
+++ b/db/post_migrate/20180723130817_delete_inconsistent_internal_id_records.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+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 do
+      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
+  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 id FROM ( -- workaround for MySQL
+       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=#{InternalId.usages.fetch(base_table)} AND maxima.maximum_iid > internal_ids.last_value
+      ) internal_ids
+    SQL
+
+    InternalId.where("id IN (#{sql})").tap do |ids| # rubocop:disable GitlabSecurity/SqlInjection
+      say "Deleting internal_id records for #{base_table}: #{ids.pluck(:project_id, :last_value)}" unless ids.empty?
+    end.delete_all
+  end
+end
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..becb71cf427
--- /dev/null
+++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+# 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(:create_models) do
+    3.times { create(scope, project: project1) }
+    3.times { create(scope, project: project2) }
+    3.times { create(scope, project: project3) }
+  end
+
+  shared_examples_for 'deleting inconsistent internal_id records' do
+    before do
+      create_models
+
+      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(:create_models) do
+      3.times { |i| create(scope, target_project: project1, source_project: project1, source_branch: i.to_s) }
+      3.times { |i| create(scope, target_project: project2, source_project: project2, source_branch: i.to_s) }
+      3.times { |i| create(scope, target_project: project3, source_project: project3, source_branch: i.to_s) }
+    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
+      3.times { create(:milestone, group: group1) }
+      3.times { create(:milestone, group: group2) }
+      3.times { 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