From b42f9a1e167f103a2cd118f678df685677da66d7 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 2 Jul 2024 15:28:13 +0300
Subject: [PATCH] Add partitioned FK from ci_pipelines_config to ci_pipelines
Changelog: other
---
..._config_on_partition_id_and_pipeline_id.rb | 38 +++++++++++++++++++
...nes_config_partition_id_and_pipeline_id.rb | 17 +++++++++
db/schema_migrations/20240702120756 | 1 +
db/schema_migrations/20240702120757 | 1 +
db/structure.sql | 3 ++
spec/db/schema_spec.rb | 1 +
...ll_partition_id_ci_pipeline_config_spec.rb | 27 ++++++++++---
7 files changed, 82 insertions(+), 6 deletions(-)
create mode 100644 db/post_migrate/20240702120756_fk_to_ci_pipelines_from_ci_pipelines_config_on_partition_id_and_pipeline_id.rb
create mode 100644 db/post_migrate/20240702120757_validate_async_fk_on_ci_pipelines_config_partition_id_and_pipeline_id.rb
create mode 100644 db/schema_migrations/20240702120756
create mode 100644 db/schema_migrations/20240702120757
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 0000000000000..5afffbd3b63cc
--- /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 0000000000000..831074456e214
--- /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/20240702120756 b/db/schema_migrations/20240702120756
new file mode 100644
index 0000000000000..87ce14c40c0d0
--- /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 0000000000000..e74852c1745bb
--- /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 b86a0c4dbbc72..3144483628197 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -34224,6 +34224,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 c708fe880da5a..7e44afe853bca 100644
--- a/spec/db/schema_spec.rb
+++ b/spec/db/schema_spec.rb
@@ -19,6 +19,7 @@
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
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 fad3e277888ef..3e3e82d07b5a1 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 }
--
GitLab