diff --git a/db/post_migrate/20240702112651_fk_to_ci_pipelines_from_ci_pipeline_metadata_on_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702112651_fk_to_ci_pipelines_from_ci_pipeline_metadata_on_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..722c988913b85c9ee41ecc9f15037d944a6164bf --- /dev/null +++ b/db/post_migrate/20240702112651_fk_to_ci_pipelines_from_ci_pipeline_metadata_on_partition_id_and_pipeline_id.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class FkToCiPipelinesFromCiPipelineMetadataOnPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :ci_pipeline_metadata + TARGET_TABLE_NAME = :ci_pipelines + COLUMN = :pipeline_id + TARGET_COLUMN = :id + FK_NAME = :fk_rails_50c1e9ea10_p + PARTITION_COLUMN = :partition_id + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [PARTITION_COLUMN, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :cascade, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end +end diff --git a/db/post_migrate/20240702112652_validate_async_fk_on_ci_pipeline_metadata_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702112652_validate_async_fk_on_ci_pipeline_metadata_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..b7decbaf31e1a22736574f32b191284f7f125b0f --- /dev/null +++ b/db/post_migrate/20240702112652_validate_async_fk_on_ci_pipeline_metadata_partition_id_and_pipeline_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class ValidateAsyncFkOnCiPipelineMetadataPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + + TABLE_NAME = :ci_pipeline_metadata + FK_NAME = :fk_rails_50c1e9ea10_p + COLUMNS = [:partition_id, :pipeline_id] + + def up + prepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + unprepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end +end diff --git a/db/post_migrate/20240702114237_fk_to_ci_pipelines_from_ci_pipeline_messages_on_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702114237_fk_to_ci_pipelines_from_ci_pipeline_messages_on_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..b01e16b9304299d304aa8dd9940f3d8d4e6f185b --- /dev/null +++ b/db/post_migrate/20240702114237_fk_to_ci_pipelines_from_ci_pipeline_messages_on_partition_id_and_pipeline_id.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class FkToCiPipelinesFromCiPipelineMessagesOnPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :ci_pipeline_messages + TARGET_TABLE_NAME = :ci_pipelines + COLUMN = :pipeline_id + TARGET_COLUMN = :id + FK_NAME = :fk_rails_8d3b04e3e1_p + PARTITION_COLUMN = :partition_id + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [PARTITION_COLUMN, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :cascade, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end +end diff --git a/db/post_migrate/20240702114238_validate_async_fk_on_ci_pipeline_messages_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702114238_validate_async_fk_on_ci_pipeline_messages_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..de56f594d509b72867f310a2cea8ae572e50bc1d --- /dev/null +++ b/db/post_migrate/20240702114238_validate_async_fk_on_ci_pipeline_messages_partition_id_and_pipeline_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class ValidateAsyncFkOnCiPipelineMessagesPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + + TABLE_NAME = :ci_pipeline_messages + FK_NAME = :fk_rails_8d3b04e3e1_p + COLUMNS = [:partition_id, :pipeline_id] + + def up + prepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + unprepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end +end diff --git a/db/post_migrate/20240702120756_fk_to_ci_pipelines_from_ci_pipelines_config_on_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702120756_fk_to_ci_pipelines_from_ci_pipelines_config_on_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..5afffbd3b63ccbda0bfb29c70d0c49ea7e0d51ce --- /dev/null +++ b/db/post_migrate/20240702120756_fk_to_ci_pipelines_from_ci_pipelines_config_on_partition_id_and_pipeline_id.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class FkToCiPipelinesFromCiPipelinesConfigOnPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :ci_pipelines_config + TARGET_TABLE_NAME = :ci_pipelines + COLUMN = :pipeline_id + TARGET_COLUMN = :id + FK_NAME = :fk_rails_906c9a2533_p + PARTITION_COLUMN = :partition_id + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [PARTITION_COLUMN, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :cascade, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end +end diff --git a/db/post_migrate/20240702120757_validate_async_fk_on_ci_pipelines_config_partition_id_and_pipeline_id.rb b/db/post_migrate/20240702120757_validate_async_fk_on_ci_pipelines_config_partition_id_and_pipeline_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..831074456e214959524a3d2dbf166b961343d166 --- /dev/null +++ b/db/post_migrate/20240702120757_validate_async_fk_on_ci_pipelines_config_partition_id_and_pipeline_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class ValidateAsyncFkOnCiPipelinesConfigPartitionIdAndPipelineId < Gitlab::Database::Migration[2.2] + milestone '17.2' + + TABLE_NAME = :ci_pipelines_config + FK_NAME = :fk_rails_906c9a2533_p + COLUMNS = [:partition_id, :pipeline_id] + + def up + prepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + unprepare_async_foreign_key_validation(TABLE_NAME, COLUMNS, name: FK_NAME) + end +end diff --git a/db/schema_migrations/20240702112651 b/db/schema_migrations/20240702112651 new file mode 100644 index 0000000000000000000000000000000000000000..bc83e72673b0e05089447cc146dbc68cb83f4f5e --- /dev/null +++ b/db/schema_migrations/20240702112651 @@ -0,0 +1 @@ +f8080dc62c1dfcc776c79aa37c6ba99425d75f745b6186f61b3df46629481ac3 \ No newline at end of file diff --git a/db/schema_migrations/20240702112652 b/db/schema_migrations/20240702112652 new file mode 100644 index 0000000000000000000000000000000000000000..2dad96b91084046fd8d27fc2cb77b375f5766c35 --- /dev/null +++ b/db/schema_migrations/20240702112652 @@ -0,0 +1 @@ +c0830183ae2d939091421eeedc1fa04074d504145b24091d5c0eb3e5d3e10b7d \ No newline at end of file diff --git a/db/schema_migrations/20240702114237 b/db/schema_migrations/20240702114237 new file mode 100644 index 0000000000000000000000000000000000000000..46c551db25aa4263ad96bbd89461455e1448c7f9 --- /dev/null +++ b/db/schema_migrations/20240702114237 @@ -0,0 +1 @@ +a490c7496dc709f0ed43c5e3caa242aaf6cd6d7e95df593f031d935708cf76b2 \ No newline at end of file diff --git a/db/schema_migrations/20240702114238 b/db/schema_migrations/20240702114238 new file mode 100644 index 0000000000000000000000000000000000000000..c7b2d525b459d015b1d391e6d02e787544667fe6 --- /dev/null +++ b/db/schema_migrations/20240702114238 @@ -0,0 +1 @@ +bdc99c187af4daca96d284526f60f0d9f58fea073e9fff4f422efb658278e7d1 \ No newline at end of file diff --git a/db/schema_migrations/20240702120756 b/db/schema_migrations/20240702120756 new file mode 100644 index 0000000000000000000000000000000000000000..87ce14c40c0d0bca74684d97d2cefba7bbe775f9 --- /dev/null +++ b/db/schema_migrations/20240702120756 @@ -0,0 +1 @@ +3b4f41807108f102864526cdb0275c3150e88e9071473c29c2873d82eceea5c2 \ No newline at end of file diff --git a/db/schema_migrations/20240702120757 b/db/schema_migrations/20240702120757 new file mode 100644 index 0000000000000000000000000000000000000000..e74852c1745bb5a3682589815cac11bc42c7c9bf --- /dev/null +++ b/db/schema_migrations/20240702120757 @@ -0,0 +1 @@ +4743abce6fbb7965a5befa24d1582fd5cadf7a73e3f757bb3d33975a97c8f5c3 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ff7806b27c8737bb2fcbabf3fc2599c7ab440eca..b6ac1d911d2e6032f50017b1dca453dd9ae6cbf5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33814,6 +33814,9 @@ ALTER TABLE ONLY status_page_settings ALTER TABLE ONLY ci_pipeline_metadata ADD CONSTRAINT fk_rails_50c1e9ea10 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE; +ALTER TABLE ONLY ci_pipeline_metadata + ADD CONSTRAINT fk_rails_50c1e9ea10_p FOREIGN KEY (partition_id, pipeline_id) REFERENCES ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY project_repository_storage_moves ADD CONSTRAINT fk_rails_5106dbd44a FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -34267,6 +34270,9 @@ ALTER TABLE ONLY vulnerability_feedback ALTER TABLE ONLY ci_pipeline_messages ADD CONSTRAINT fk_rails_8d3b04e3e1 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE; +ALTER TABLE ONLY ci_pipeline_messages + ADD CONSTRAINT fk_rails_8d3b04e3e1_p FOREIGN KEY (partition_id, pipeline_id) REFERENCES ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; + ALTER TABLE incident_management_pending_alert_escalations ADD CONSTRAINT fk_rails_8d8de95da9 FOREIGN KEY (alert_id) REFERENCES alert_management_alerts(id) ON DELETE CASCADE; @@ -34294,6 +34300,9 @@ ALTER TABLE ONLY organization_details ALTER TABLE ONLY ci_pipelines_config ADD CONSTRAINT fk_rails_906c9a2533 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE; +ALTER TABLE ONLY ci_pipelines_config + ADD CONSTRAINT fk_rails_906c9a2533_p FOREIGN KEY (partition_id, pipeline_id) REFERENCES ci_pipelines(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY approval_project_rules_groups ADD CONSTRAINT fk_rails_9071e863d1 FOREIGN KEY (approval_project_rule_id) REFERENCES approval_project_rules(id) ON DELETE CASCADE; diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index c4994b90e12886f5f3e3cbeeae2869b93521c8a5..d253db61ff5bd686b1c2a02ab6d105fd97ee05b3 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -20,6 +20,9 @@ users: [%w[accepted_term_id]], ci_builds: [%w[partition_id stage_id], %w[partition_id execution_config_id], %w[partition_id upstream_pipeline_id], %w[auto_canceled_by_partition_id auto_canceled_by_id], %w[partition_id commit_id]], # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142804#note_1745483081 ci_pipeline_variables: [%w[partition_id pipeline_id]], # index on pipeline_id is sufficient + ci_pipelines_config: [%w[partition_id pipeline_id]], # index on pipeline_id is sufficient + ci_pipeline_metadata: [%w[partition_id pipeline_id]], # index on pipeline_id is sufficient + ci_pipeline_messages: [%w[partition_id pipeline_id]], # index on pipeline_id is sufficient p_ci_builds: [%w[partition_id stage_id], %w[partition_id execution_config_id]], # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142804#note_1745483081 ci_stages: [%w[partition_id pipeline_id]], # the index on pipeline_id is sufficient ai_testing_terms_acceptances: %w[user_id], # testing terms only have 1 entry, and if the user is deleted the record should remain diff --git a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_config_spec.rb index fad3e277888ef5926089edbc06b202a5b4dbf9cc..3e3e82d07b5a174e87c62c07d3ec4dd231ca8411 100644 --- a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_config_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_config_spec.rb @@ -8,7 +8,7 @@ let(:ci_pipeline_config_table) { table(:ci_pipelines_config, database: :ci) } let!(:pipeline_1) { ci_pipelines_table.create!(id: 1, partition_id: 100) } let!(:pipeline_2) { ci_pipelines_table.create!(id: 2, partition_id: 101) } - let!(:pipeline_3) { ci_pipelines_table.create!(id: 3, partition_id: 101) } + let!(:pipeline_3) { ci_pipelines_table.create!(id: 3, partition_id: 100) } let!(:ci_pipeline_config_100) do ci_pipeline_config_table.create!( pipeline_id: pipeline_1.id, @@ -41,18 +41,29 @@ batch_column: :pipeline_id, sub_batch_size: 1, pause_ms: 0, - connection: Ci::ApplicationRecord.connection + connection: connection } end let!(:migration) { described_class.new(**migration_attrs) } + let(:connection) { Ci::ApplicationRecord.connection } + + around do |example| + connection.transaction do + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines DISABLE TRIGGER ALL; + SQL + + example.run + + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines ENABLE TRIGGER ALL; + SQL + end + end describe '#perform' do context 'when second partition does not exist' do - before do - pipeline_3.update!(partition_id: 100) - end - it 'does not execute the migration' do expect { migration.perform } .not_to change { invalid_ci_pipeline_config.reload.partition_id } @@ -60,6 +71,10 @@ end context 'when second partition exists' do + before do + pipeline_3.update!(partition_id: 101) + end + it 'fixes invalid records in the wrong the partition' do expect { migration.perform } .to not_change { ci_pipeline_config_100.reload.partition_id } diff --git a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_message_spec.rb b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_message_spec.rb index d40b19c6733a7eaa587f2efcd27da6f4d4957b7a..a7643513cb9284ef95708bbc0add3c867e39d914 100644 --- a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_message_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_message_spec.rb @@ -7,7 +7,7 @@ let(:ci_pipeline_messages_table) { table(:ci_pipeline_messages, database: :ci) } let!(:pipeline_1) { ci_pipelines_table.create!(id: 1, partition_id: 100) } let!(:pipeline_2) { ci_pipelines_table.create!(id: 2, partition_id: 101) } - let!(:pipeline_3) { ci_pipelines_table.create!(id: 3, partition_id: 101) } + let!(:pipeline_3) { ci_pipelines_table.create!(id: 3, partition_id: 100) } let!(:ci_pipeline_messages_100) do ci_pipeline_messages_table.create!( content: 'content', @@ -28,7 +28,7 @@ ci_pipeline_messages_table.create!( content: 'content', pipeline_id: pipeline_3.id, - partition_id: pipeline_1.partition_id + partition_id: pipeline_3.partition_id ) end @@ -40,18 +40,29 @@ batch_column: :id, sub_batch_size: 1, pause_ms: 0, - connection: Ci::ApplicationRecord.connection + connection: connection } end let!(:migration) { described_class.new(**migration_attrs) } + let(:connection) { Ci::ApplicationRecord.connection } + + around do |example| + connection.transaction do + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines DISABLE TRIGGER ALL; + SQL + + example.run + + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines ENABLE TRIGGER ALL; + SQL + end + end describe '#perform' do context 'when there are no invalid records' do - before do - pipeline_3.update!(partition_id: 100) - end - it 'does not execute the migration' do expect { migration.perform } .not_to change { invalid_ci_pipeline_messages.reload.partition_id } @@ -59,6 +70,10 @@ end context 'when second partition exists' do + before do + pipeline_3.update!(partition_id: 101) + end + it 'fixes invalid records in the wrong the partition' do expect { migration.perform } .to not_change { ci_pipeline_messages_100.reload.partition_id } diff --git a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_metadata_spec.rb b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_metadata_spec.rb index d09d5016dcc0db77734e3d34969a479c692e1d02..dcf3affdad71d53c2c8a37cc835f6c3cc18f3b1c 100644 --- a/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_metadata_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_partition_id_ci_pipeline_metadata_spec.rb @@ -8,7 +8,7 @@ let(:ci_pipeline_metadata_table) { table(:ci_pipeline_metadata, database: :ci) } let!(:pipeline_100) { ci_pipelines_table.create!(id: 1, partition_id: 100) } let!(:pipeline_101) { ci_pipelines_table.create!(id: 2, partition_id: 101) } - let!(:pipeline_102) { ci_pipelines_table.create!(id: 3, partition_id: 101) } + let!(:pipeline_102) { ci_pipelines_table.create!(id: 3, partition_id: 100) } let!(:ci_pipeline_metadata_100) do ci_pipeline_metadata_table.create!( pipeline_id: pipeline_100.id, @@ -29,7 +29,7 @@ ci_pipeline_metadata_table.create!( pipeline_id: pipeline_102.id, project_id: 1, - partition_id: pipeline_100.partition_id + partition_id: pipeline_102.partition_id ) end @@ -46,6 +46,21 @@ end let!(:migration) { described_class.new(**migration_attrs) } + let(:connection) { Ci::ApplicationRecord.connection } + + around do |example| + connection.transaction do + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines DISABLE TRIGGER ALL; + SQL + + example.run + + connection.execute(<<~SQL) + ALTER TABLE ci_pipelines ENABLE TRIGGER ALL; + SQL + end + end describe '#perform' do context 'when second partition does not exist' do @@ -58,6 +73,7 @@ context 'when second partition exists' do before do allow(migration).to receive(:uses_multiple_partitions?).and_return(true) + pipeline_102.update!(partition_id: 101) end it 'fixes invalid records in the wrong the partition' do