From 106b02401b3e9c556c47d36404a308dda7a05bc3 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 6 Mar 2023 11:00:45 +0200 Subject: [PATCH 1/2] Validate and replace FK for p_ci_builds_metadata and ci_builds It synchronously validates the new foreign key between the partitions of p_ci_builds_metadata and ci_builds in preparation of adding the foreign key on the routing table. It also removes the obsolete FK on the single column. Changelog: other --- ...g_fk_on_p_ci_builds_metadata_partitions.rb | 18 ++++++++++ ...s_metadata_on_partition_id_and_build_id.rb | 36 +++++++++++++++++++ ...builds_p_ci_builds_metadata_on_build_id.rb | 32 +++++++++++++++++ db/schema_migrations/20230306071456 | 1 + db/schema_migrations/20230306072532 | 1 + db/schema_migrations/20230306082852 | 1 + db/structure.sql | 5 +-- spec/db/schema_spec.rb | 5 +-- 8 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb create mode 100644 db/post_migrate/20230306072532_add_partitioned_fk_to_p_ci_builds_metadata_on_partition_id_and_build_id.rb create mode 100644 db/post_migrate/20230306082852_remove_fk_to_ci_builds_p_ci_builds_metadata_on_build_id.rb create mode 100644 db/schema_migrations/20230306071456 create mode 100644 db/schema_migrations/20230306072532 create mode 100644 db/schema_migrations/20230306082852 diff --git a/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb b/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb new file mode 100644 index 00000000000000..5437626e47a294 --- /dev/null +++ b/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class ValidatePartitioningFkOnPCiBuildsMetadataPartitions < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :p_ci_builds_metadata + FK_NAME = :fk_e20479742e_p + + def up + Gitlab::Database::PostgresPartitionedTable.each_partition(SOURCE_TABLE_NAME) do |partition| + validate_foreign_key(partition.identifier, nil, name: FK_NAME) + end + end + + def down + # No-op + end +end diff --git a/db/post_migrate/20230306072532_add_partitioned_fk_to_p_ci_builds_metadata_on_partition_id_and_build_id.rb b/db/post_migrate/20230306072532_add_partitioned_fk_to_p_ci_builds_metadata_on_partition_id_and_build_id.rb new file mode 100644 index 00000000000000..9328e3ff00e04c --- /dev/null +++ b/db/post_migrate/20230306072532_add_partitioned_fk_to_p_ci_builds_metadata_on_partition_id_and_build_id.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class AddPartitionedFkToPCiBuildsMetadataOnPartitionIdAndBuildId < Gitlab::Database::Migration[2.1] + SOURCE_TABLE_NAME = :p_ci_builds_metadata + TARGET_TABLE_NAME = :ci_builds + FK_NAME = :fk_e20479742e_p + + disable_ddl_transaction! + + def up + return if foreign_key_exists?(SOURCE_TABLE_NAME, TARGET_TABLE_NAME, name: FK_NAME) + + with_lock_retries do + execute("LOCK TABLE #{TARGET_TABLE_NAME}, #{SOURCE_TABLE_NAME} IN SHARE ROW EXCLUSIVE MODE") + + execute(<<~SQL.squish) + ALTER TABLE #{SOURCE_TABLE_NAME} + ADD CONSTRAINT #{FK_NAME} + FOREIGN KEY (partition_id, build_id) + REFERENCES #{TARGET_TABLE_NAME} (partition_id, id) + ON UPDATE CASCADE ON DELETE CASCADE; + SQL + end + 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/20230306082852_remove_fk_to_ci_builds_p_ci_builds_metadata_on_build_id.rb b/db/post_migrate/20230306082852_remove_fk_to_ci_builds_p_ci_builds_metadata_on_build_id.rb new file mode 100644 index 00000000000000..108a92aec3b7a1 --- /dev/null +++ b/db/post_migrate/20230306082852_remove_fk_to_ci_builds_p_ci_builds_metadata_on_build_id.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class RemoveFkToCiBuildsPCiBuildsMetadataOnBuildId < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :p_ci_builds_metadata + TARGET_TABLE_NAME = :ci_builds + FK_NAME = :fk_e20479742e + + def up + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end + + def down + add_concurrent_partitioned_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: :build_id, + on_delete: :cascade, + name: FK_NAME + ) + end +end diff --git a/db/schema_migrations/20230306071456 b/db/schema_migrations/20230306071456 new file mode 100644 index 00000000000000..b4ac086f12591f --- /dev/null +++ b/db/schema_migrations/20230306071456 @@ -0,0 +1 @@ +7f431d6dd4f9dc237623c18465995fa59c9902187f433375baa03194f7a6b88f \ No newline at end of file diff --git a/db/schema_migrations/20230306072532 b/db/schema_migrations/20230306072532 new file mode 100644 index 00000000000000..f1604aa84a7b2b --- /dev/null +++ b/db/schema_migrations/20230306072532 @@ -0,0 +1 @@ +f6613d1fd3b99fa0e8ea059c6d53e8d226ce3fd8c07e44a024b065d8d110876f \ No newline at end of file diff --git a/db/schema_migrations/20230306082852 b/db/schema_migrations/20230306082852 new file mode 100644 index 00000000000000..bbbe7cb27ef45e --- /dev/null +++ b/db/schema_migrations/20230306082852 @@ -0,0 +1 @@ +580efa96f235c47de1bcea172544e51e8207dd0a81bd888567b30ce02e453f7d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ceddf2a82a5e9a..f16d4757e95f79 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -34838,10 +34838,7 @@ ALTER TABLE ONLY ci_sources_pipelines ADD CONSTRAINT fk_e1bad85861 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE; ALTER TABLE p_ci_builds_metadata - ADD CONSTRAINT fk_e20479742e FOREIGN KEY (build_id) REFERENCES ci_builds(id) ON DELETE CASCADE; - -ALTER TABLE ONLY ci_builds_metadata - ADD CONSTRAINT fk_e20479742e_p FOREIGN KEY (partition_id, build_id) REFERENCES ci_builds(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; + ADD CONSTRAINT fk_e20479742e_p FOREIGN KEY (partition_id, build_id) REFERENCES ci_builds(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE; ALTER TABLE ONLY gitlab_subscriptions ADD CONSTRAINT fk_e2595d00a1 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index bd9e9f1c4cf141..69eae1c00db884 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -10,7 +10,8 @@ let(:columns_name_with_jsonb) { retrieve_columns_name_with_jsonb } IGNORED_INDEXES_ON_FKS = { - slack_integrations_scopes: %w[slack_api_scope_id] + slack_integrations_scopes: %w[slack_api_scope_id], + p_ci_builds_metadata: %w[partition_id] # composable FK, the columns are reversed in the index definition }.with_indifferent_access.freeze TABLE_PARTITIONS = %w[ci_builds_metadata].freeze @@ -39,7 +40,7 @@ ci_build_trace_metadata: %w[partition_id build_id], ci_builds: %w[erased_by_id trigger_request_id partition_id], ci_builds_runner_session: %w[partition_id build_id], - p_ci_builds_metadata: %w[partition_id], + p_ci_builds_metadata: %w[partition_id build_id], ci_job_artifacts: %w[partition_id job_id], ci_job_variables: %w[partition_id job_id], ci_namespace_monthly_usages: %w[namespace_id], -- GitLab From 424e72d6abbd90a0f0791f496d97d3beb3bc1db3 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 6 Mar 2023 15:10:54 +0200 Subject: [PATCH 2/2] Apply review feedback --- ...r_p_ci_builds_metadata_partitions_and_ci_builds.rb | 11 +++++++---- ...titioning_fk_on_p_ci_builds_metadata_partitions.rb | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20230223082752_schedule_fk_validation_for_p_ci_builds_metadata_partitions_and_ci_builds.rb b/db/post_migrate/20230223082752_schedule_fk_validation_for_p_ci_builds_metadata_partitions_and_ci_builds.rb index 583a9cd31f7a3d..bcb1147605ed35 100644 --- a/db/post_migrate/20230223082752_schedule_fk_validation_for_p_ci_builds_metadata_partitions_and_ci_builds.rb +++ b/db/post_migrate/20230223082752_schedule_fk_validation_for_p_ci_builds_metadata_partitions_and_ci_builds.rb @@ -1,14 +1,17 @@ # frozen_string_literal: true class ScheduleFkValidationForPCiBuildsMetadataPartitionsAndCiBuilds < Gitlab::Database::Migration[2.1] - TABLE_NAME = :p_ci_builds_metadata - FK_NAME = :fk_e20479742e_p + # This migration was used to validate the foreign keys on partitions introduced by + # db/post_migrate/20230221125148_add_fk_to_p_ci_builds_metadata_partitions_on_partition_id_and_build_id.rb + # but executing the rollback of + # db/post_migrate/20230306072532_add_partitioned_fk_to_p_ci_builds_metadata_on_partition_id_and_build_id.rb + # would also remove the FKs on partitions and this would errors out. def up - prepare_partitioned_async_foreign_key_validation TABLE_NAME, name: FK_NAME + # No-op end def down - unprepare_partitioned_async_foreign_key_validation TABLE_NAME, name: FK_NAME + # No-op end end diff --git a/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb b/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb index 5437626e47a294..f07175e82f9a34 100644 --- a/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb +++ b/db/post_migrate/20230306071456_validate_partitioning_fk_on_p_ci_builds_metadata_partitions.rb @@ -8,6 +8,8 @@ class ValidatePartitioningFkOnPCiBuildsMetadataPartitions < Gitlab::Database::Mi def up Gitlab::Database::PostgresPartitionedTable.each_partition(SOURCE_TABLE_NAME) do |partition| + next unless foreign_key_exists?(partition.identifier, name: FK_NAME) + validate_foreign_key(partition.identifier, nil, name: FK_NAME) end end -- GitLab