From 45760de0eade1e3146d39355caea559e039bc670 Mon Sep 17 00:00:00 2001 From: Andreas Brandl Date: Mon, 23 Jul 2018 16:36:21 +0200 Subject: Add migration to cleanup internal_ids. See https://gitlab.com/gitlab-org/gitlab-ce/issues/49446. --- .../ab-49446-internal-ids-inconsistency.yml | 5 + ...0817_delete_inconsistent_internal_id_records.rb | 54 +++++++++ db/schema.rb | 2 +- ...delete_inconsistent_internal_id_records_spec.rb | 129 +++++++++++++++++++++ 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/ab-49446-internal-ids-inconsistency.yml create mode 100644 db/post_migrate/20180723130817_delete_inconsistent_internal_id_records.rb create mode 100644 spec/migrations/delete_inconsistent_internal_id_records_spec.rb 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 -- cgit v1.2.1