From 4ae5f9d22448b3dc5cc48f072ddf0e098b29a132 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 29 Aug 2025 15:09:18 -0700 Subject: [PATCH] Rename misnamed CI indexes The database diagnostics page flagged a number of indexes that were not consistent with `db/structure.sql` due to the bigint conversion. This commit renames 14 CI indexes from and to the following names: 1. p_ci_builds_metadata_build_id_convert_to_bigint_id_convert__idx -> p_ci_builds_metadata_build_id_id_idx 2. p_ci_builds_metadata_build_id_convert_to_bigint_idx -> p_ci_builds_metadata_build_id_idx 3. p_ci_pipelines_ci_ref_id_id_convert_to_bigint_idx -> p_ci_pipelines_ci_ref_id_id_idx 4. p_ci_pipelines_ci_ref_id_id_convert_to_bigint_source_status_idx -> p_ci_pipelines_ci_ref_id_id_source_status_idx 5. p_ci_pipelines_id_convert_to_bigint_idx -> p_ci_pipelines_id_idx 6. p_ci_pipelines_pipeline_schedule_id_id_convert_to_bigint_idx ->p_ci_pipelines_pipeline_schedule_id_id_idx 7. p_ci_pipelines_project_id_id_convert_to_bigint_idx -> p_ci_pipelines_project_id_id_idx 8. p_ci_pipelines_project_id_ref_id_convert_to_bigint_idx ->p_ci_pipelines_project_id_ref_id_idx 9. p_ci_pipelines_project_id_ref_status_id_convert_to_bigint_idx ->p_ci_pipelines_project_id_ref_status_id_idx 10. p_ci_pipelines_status_id_convert_to_bigint_idx -> p_ci_pipelines_status_id_idx 11. p_ci_pipelines_user_id_id_convert_to_bigint_idx -> p_ci_pipelines_user_id_id_idx 12. p_ci_pipelines_user_id_id_convert_to_bigint_idx1 -> p_ci_pipelines_user_id_id_idx1 13. p_ci_stages_pipeline_id_convert_to_bigint_id_idx -> p_ci_stages_pipeline_id_id_idx 14. p_ci_stages_pipeline_id_convert_to_bigint_position_idx -> p_ci_stages_pipeline_id_position_idx Relates to https://gitlab.com/gitlab-com/request-for-help/-/issues/3342 --- .../20250829201711_fix_misnamed_ci_indexes.rb | 56 +++++++++++ db/schema_migrations/20250829201711 | 1 + ...0829201711_fix_misnamed_ci_indexes_spec.rb | 97 +++++++++++++++++++ 3 files changed, 154 insertions(+) create mode 100644 db/post_migrate/20250829201711_fix_misnamed_ci_indexes.rb create mode 100644 db/schema_migrations/20250829201711 create mode 100644 spec/migrations/20250829201711_fix_misnamed_ci_indexes_spec.rb diff --git a/db/post_migrate/20250829201711_fix_misnamed_ci_indexes.rb b/db/post_migrate/20250829201711_fix_misnamed_ci_indexes.rb new file mode 100644 index 00000000000000..9d7bd749dacbba --- /dev/null +++ b/db/post_migrate/20250829201711_fix_misnamed_ci_indexes.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +class FixMisnamedCiIndexes < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.4' + + # Mapping of table names -> [index columns, old name, expected name] + # rubocop:disable Layout/LineLength -- More readable on single line + INDEXES_TO_RENAME = { + 'p_ci_builds_metadata' => [ + %w[build_id p_ci_builds_metadata_build_id_convert_to_bigint_id_convert__idx p_ci_builds_metadata_build_id_id_idx], + %w[build_id p_ci_builds_metadata_build_id_convert_to_bigint_idx p_ci_builds_metadata_build_id_idx] + ], + 'p_ci_pipelines' => [ + [%w[ci_ref_id id], 'p_ci_pipelines_ci_ref_id_id_convert_to_bigint_idx', 'p_ci_pipelines_ci_ref_id_id_idx'], + [%w[ci_ref_id id source status], 'p_ci_pipelines_ci_ref_id_id_convert_to_bigint_source_status_idx', 'p_ci_pipelines_ci_ref_id_id_source_status_idx'], + [%w[id], 'p_ci_pipelines_id_convert_to_bigint_idx', 'p_ci_pipelines_id_idx'], + [%w[pipeline_schedule_id id], 'p_ci_pipelines_pipeline_schedule_id_id_convert_to_bigint_idx', 'p_ci_pipelines_pipeline_schedule_id_id_idx'], + [%w[project_id id], 'p_ci_pipelines_project_id_id_convert_to_bigint_idx', 'p_ci_pipelines_project_id_id_idx'], + [%w[project_id ref id], 'p_ci_pipelines_project_id_ref_id_convert_to_bigint_idx', 'p_ci_pipelines_project_id_ref_id_idx'], + [%w[project_id ref status id], 'p_ci_pipelines_project_id_ref_status_id_convert_to_bigint_idx', 'p_ci_pipelines_project_id_ref_status_id_idx'], + [%w[status id], 'p_ci_pipelines_status_id_convert_to_bigint_idx', 'p_ci_pipelines_status_id_idx'], + [%w[user_id id], 'p_ci_pipelines_user_id_id_convert_to_bigint_idx', 'p_ci_pipelines_user_id_id_idx'], + [%w[user_id id], 'p_ci_pipelines_user_id_id_convert_to_bigint_idx1', 'p_ci_pipelines_user_id_id_idx1'] + ], + 'p_ci_stages' => [ + [%w[pipeline_id id], 'p_ci_stages_pipeline_id_convert_to_bigint_id_idx', 'p_ci_stages_pipeline_id_id_idx'], + [%w[pipeline_id position], 'p_ci_stages_pipeline_id_convert_to_bigint_position_idx', 'p_ci_stages_pipeline_id_position_idx'] + ] + }.freeze + # rubocop:enable Layout/LineLength + + def up + with_each_index do |table, columns, old_name, new_name| + next if index_exists?(table, columns, name: new_name) + next unless index_exists?(table, columns, name: old_name) + + with_lock_retries { rename_index(table, old_name, new_name) } + end + end + + # We have no way of tracking which misnamed indexes existed before the migration, + # so let's not try to restore the previous, incorrect state. + def down; end + + private + + def with_each_index + INDEXES_TO_RENAME.each do |table, indexes| + indexes.each do |index_info| + columns, old_name, new_name = index_info + yield table, columns, old_name, new_name + end + end + end +end diff --git a/db/schema_migrations/20250829201711 b/db/schema_migrations/20250829201711 new file mode 100644 index 00000000000000..2f7990c7833253 --- /dev/null +++ b/db/schema_migrations/20250829201711 @@ -0,0 +1 @@ +852730e604c12d0e288aac5f616070bda945e3a07cea0f7db7d38b0567938bff \ No newline at end of file diff --git a/spec/migrations/20250829201711_fix_misnamed_ci_indexes_spec.rb b/spec/migrations/20250829201711_fix_misnamed_ci_indexes_spec.rb new file mode 100644 index 00000000000000..2677f8912ccbbf --- /dev/null +++ b/spec/migrations/20250829201711_fix_misnamed_ci_indexes_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe FixMisnamedCiIndexes, feature_category: :continuous_integration do + let(:migration) { described_class.new } + + # Table references for the partitioned tables + let(:ci_builds_metadata) { table(:p_ci_builds_metadata) } + let(:ci_pipelines) { table(:p_ci_pipelines) } + let(:ci_stages) { table(:p_ci_stages) } + + describe '#up' do + before do + # Create the old indexes that need to be renamed + described_class::INDEXES_TO_RENAME.each do |table_name, indexes| + indexes.each do |columns, old_name, new_name| + next if migration.index_exists?(table_name, columns, name: old_name) + + migration.rename_index(table_name, new_name, old_name) + end + end + end + + it 'renames all misnamed indexes to their correct names' do + # Sanity check: verify old indexes exist before migration + described_class::INDEXES_TO_RENAME.each do |table_name, indexes| + indexes.each do |columns, old_name, _new_name| + expect(migration.index_exists?(table_name, columns, name: old_name)) + .to be(true), "Expected old index #{old_name} to exist on #{table_name} before migration" + end + end + + migrate! + + # Verify all indexes were renamed correctly + described_class::INDEXES_TO_RENAME.each do |table_name, indexes| + indexes.each do |columns, old_name, new_name| + expect(migration.index_exists?(table_name, columns, name: old_name)) + .to be(false), "Expected old index #{old_name} to not exist on #{table_name} after migration" + + expect(migration.index_exists?(table_name, columns, name: new_name)) + .to be(true), "Expected new index #{new_name} to exist on #{table_name} after migration" + end + end + end + + it 'is idempotent and can be run multiple times safely' do + migrate! + + expect { migrate! }.not_to raise_error + + # Verify indexes are still correctly named + described_class::INDEXES_TO_RENAME.each do |table_name, indexes| + indexes.each do |columns, _old_name, new_name| + expect(migration.index_exists?(table_name, columns, name: new_name)) + .to be(true), "Expected new index #{new_name} to still exist after second migration run" + end + end + end + + it 'skips renaming if the new index already exists' do + # Create some new indexes manually + sample_table, sample_indexes = described_class::INDEXES_TO_RENAME.first + sample_columns, _, sample_new_name = sample_indexes.first + + # Create the new index manually + migration.add_index(sample_table, sample_columns, name: sample_new_name) + + expect { migrate! }.not_to raise_error + + # Verify the manually created index still exists + expect(migration.index_exists?(sample_table, sample_columns, name: sample_new_name)) + .to be(true) + + # Drop this newly added index for subsequent tests + migration.remove_index(sample_table, sample_columns, name: sample_new_name) + end + + it 'skips renaming if the old index does not exist' do + # Remove one of the old indexes + sample_table, sample_indexes = described_class::INDEXES_TO_RENAME.first + sample_columns, sample_old_name, sample_new_name = sample_indexes.first + + if migration.index_exists?(sample_table, sample_columns, name: sample_old_name) + migration.remove_index(sample_table, sample_columns, name: sample_old_name) + end + + expect { migrate! }.not_to raise_error + + # Verify the new index was not created since old one didn't exist + expect(migration.index_exists?(sample_table, sample_columns, name: sample_new_name)) + .to be(false) + end + end +end -- GitLab