From 5fb90a3073383abdbecf1670537f8f788c0ccc5c Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Tue, 11 Nov 2025 17:47:45 +1100 Subject: [PATCH 1/5] Prepare foreign keys for bigint conversion on merge_requests Stage Two --- ...es_creation_on_merge_requests_stage_two.rb | 107 ++++++++++++++++++ ...oreign_keys_on_merge_requests_stage_two.rb | 80 +++++++++++++ db/schema_migrations/20251111050757 | 1 + db/schema_migrations/20251111050850 | 1 + 4 files changed, 189 insertions(+) create mode 100644 db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb create mode 100644 db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb create mode 100644 db/schema_migrations/20251111050757 create mode 100644 db/schema_migrations/20251111050850 diff --git a/db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb b/db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb new file mode 100644 index 00000000000000..b9f8d43f6c868e --- /dev/null +++ b/db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +class SyncBigintIndexesCreationOnMergeRequestsStageTwo < Gitlab::Database::Migration[2.3] + include Gitlab::Database::MigrationHelpers::ConvertToBigint + + disable_ddl_transaction! + milestone '18.7' + + TABLE_NAME = 'merge_requests' + BIGINT_COLUMNS = [ + :target_project_id_convert_to_bigint, + :latest_merge_request_diff_id_convert_to_bigint, + :last_edited_by_id_convert_to_bigint + ].freeze + INDEXES = [ + { + name: 'index_merge_requests_on_target_project_id_and_iid', + columns: [:target_project_id_convert_to_bigint, :iid], + options: { unique: true } + }, + { + name: 'index_merge_requests_on_target_project_id_and_merged_commit_sha', + columns: [:target_project_id_convert_to_bigint, :merged_commit_sha] + }, + { + name: 'index_merge_requests_on_target_project_id_and_source_branch', + columns: [:target_project_id_convert_to_bigint, :source_branch] + }, + { + name: 'index_merge_requests_on_target_project_id_and_squash_commit_sha', + columns: [:target_project_id_convert_to_bigint, :squash_commit_sha] + }, + { + name: 'index_merge_requests_on_target_project_id_and_target_branch', + columns: [:target_project_id_convert_to_bigint, :target_branch], + options: { where: "state_id = 1 AND merge_when_pipeline_succeeds = true" } + }, + { + name: 'index_merge_requests_for_latest_diffs_with_state_merged', + columns: [:latest_merge_request_diff_id_convert_to_bigint, :target_project_id_convert_to_bigint], + options: { where: "state_id = 3" } + }, + { + name: 'index_merge_requests_on_latest_merge_request_diff_id', + columns: [:latest_merge_request_diff_id_convert_to_bigint] + }, + { + name: 'idx_mrs_on_target_id_and_created_at_and_state_id', + columns: [:target_project_id_convert_to_bigint, :state_id, :created_at, :id] + }, + { + name: 'index_merge_requests_on_target_project_id_and_created_at_and_id', + columns: [:target_project_id_convert_to_bigint, :created_at, :id] + }, + { + name: 'index_merge_requests_on_target_project_id_and_updated_at_and_id', + columns: [:target_project_id_convert_to_bigint, :updated_at, :id] + }, + { + name: 'index_merge_requests_on_tp_id_and_merge_commit_sha_and_id', + columns: [:target_project_id_convert_to_bigint, :merge_commit_sha, :id] + }, + { + name: 'index_merge_requests_on_author_id_and_target_project_id', + columns: [:author_id, :target_project_id_convert_to_bigint] + }, + { + name: 'index_on_merge_requests_for_latest_diffs', + columns: [:target_project_id_convert_to_bigint], + options: { include: [:id, :latest_merge_request_diff_id_convert_to_bigint] } + } + ].freeze + + def up + return if skip_migration? + + # rubocop:disable Migration/PreventIndexCreation -- bigint migration + INDEXES.each do |index| + options = index[:options] || {} + add_concurrent_index TABLE_NAME, index[:columns], name: bigint_index_name(index[:name]), **options + end + # rubocop:enable Migration/PreventIndexCreation + end + + def down + return if skip_migration? + + INDEXES.each do |index| + remove_concurrent_index_by_name TABLE_NAME, bigint_index_name(index[:name]) + end + end + + private + + def skip_migration? + unless conversion_columns_exist? + say "No conversion columns found - migration skipped" + return true + end + + false + end + + def conversion_columns_exist? + BIGINT_COLUMNS.all? { |column| column_exists?(TABLE_NAME, column) } + end +end diff --git a/db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb b/db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb new file mode 100644 index 00000000000000..a50c9d8f2ba87c --- /dev/null +++ b/db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.7' + + TABLE_NAME = 'merge_requests' + COLUMNS = %i[target_project_id latest_merge_request_diff_id last_edited_by_id].freeze + FOREIGN_KEYS = [ + { + source_table: :merge_requests, + column: :target_project_id_convert_to_bigint, + target_table: :projects, + target_column: :id, + on_delete: :cascade, + name: :fk_a6963e8447 + }, + { + source_table: :merge_requests, + column: :latest_merge_request_diff_id_convert_to_bigint, + target_table: :merge_request_diffs, + target_column: :id, + on_delete: :nullify, + name: :fk_06067f5644 + }, + { + source_table: :p_generated_ref_commits, + column: [:project_id, :merge_request_iid], + target_table: :merge_requests, + target_column: [:target_project_id_convert_to_bigint, :iid], + on_delete: :cascade, + name: :fk_generated_ref_commits_merge_request_id + } + ].freeze + + def up + conversion_needed = COLUMNS.all? do |column| + column_exists?(TABLE_NAME, convert_to_bigint_column(column)) + end + + unless conversion_needed + say "No conversion columns found - no need to create bigint FKs" + return + end + + FOREIGN_KEYS.each do |fk| + add_concurrent_foreign_key( + fk[:source_table], + fk[:target_table], + column: fk[:column], + target_column: fk[:target_column], + name: tmp_name(fk[:name]), + on_delete: fk[:on_delete], + validate: false, + reverse_lock_order: true + ) + + prepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) + end + end + + def down + FOREIGN_KEYS.each do |fk| + remove_foreign_key_if_exists( + fk[:source_table], + fk[:target_table], + name: tmp_name(fk[:name]), + reverse_lock_order: true + ) + + unprepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) + end + end + + private + + def tmp_name(name) + "#{name}_tmp" + end +end diff --git a/db/schema_migrations/20251111050757 b/db/schema_migrations/20251111050757 new file mode 100644 index 00000000000000..5fe0dbd52d7d2d --- /dev/null +++ b/db/schema_migrations/20251111050757 @@ -0,0 +1 @@ +c8edbaacce8a080220aa7e8416743928415c5b1bc201de0f0eaa87f3428451b7 \ No newline at end of file diff --git a/db/schema_migrations/20251111050850 b/db/schema_migrations/20251111050850 new file mode 100644 index 00000000000000..a69ee04e1b16b9 --- /dev/null +++ b/db/schema_migrations/20251111050850 @@ -0,0 +1 @@ +ca0f87aa98ef4f199e69cdbca7398e0738b4357e821b6305406c842cb0e05909 \ No newline at end of file -- GitLab From 9ecb00155104834f39787f39c28be7da2cd7b90c Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 1 Dec 2025 10:34:03 +1100 Subject: [PATCH 2/5] refresh timestamp & update partitioned fk helper --- ...s_creation_on_merge_requests_stage_two.rb} | 0 ...reign_keys_on_merge_requests_stage_two.rb} | 25 ++++++++++++++++++- db/schema_migrations/20251111050757 | 1 - db/schema_migrations/20251111050850 | 1 - db/schema_migrations/20251130050757 | 1 + db/schema_migrations/20251130050850 | 1 + 6 files changed, 26 insertions(+), 3 deletions(-) rename db/post_migrate/{20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb => 20251130050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb} (100%) rename db/post_migrate/{20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb => 20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb} (78%) delete mode 100644 db/schema_migrations/20251111050757 delete mode 100644 db/schema_migrations/20251111050850 create mode 100644 db/schema_migrations/20251130050757 create mode 100644 db/schema_migrations/20251130050850 diff --git a/db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb b/db/post_migrate/20251130050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb similarity index 100% rename from db/post_migrate/20251111050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb rename to db/post_migrate/20251130050757_sync_bigint_indexes_creation_on_merge_requests_stage_two.rb diff --git a/db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb similarity index 78% rename from db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb rename to db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb index a50c9d8f2ba87c..044164bc4f732c 100644 --- a/db/post_migrate/20251111050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb +++ b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb @@ -22,7 +22,9 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_column: :id, on_delete: :nullify, name: :fk_06067f5644 - }, + } + ].freeze + PARTITIONED_FOREIGN_KEYS = [ { source_table: :p_generated_ref_commits, column: [:project_id, :merge_request_iid], @@ -57,9 +59,30 @@ def up prepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) end + + PARTITIONED_FOREIGN_KEYS.each do |fk| + add_concurrent_partitioned_foreign_key( + fk[:source_table], + fk[:target_table], + column: fk[:column], + target_column: fk[:target_column], + name: tmp_name(fk[:name]), + on_delete: fk[:on_delete], + reverse_lock_order: true + ) + end end def down + PARTITIONED_FOREIGN_KEYS.each do |fk| + remove_partitioned_foreign_key( + fk[:source_table], + fk[:target_table], + name: tmp_name(fk[:name]), + reverse_lock_order: true + ) + end + FOREIGN_KEYS.each do |fk| remove_foreign_key_if_exists( fk[:source_table], diff --git a/db/schema_migrations/20251111050757 b/db/schema_migrations/20251111050757 deleted file mode 100644 index 5fe0dbd52d7d2d..00000000000000 --- a/db/schema_migrations/20251111050757 +++ /dev/null @@ -1 +0,0 @@ -c8edbaacce8a080220aa7e8416743928415c5b1bc201de0f0eaa87f3428451b7 \ No newline at end of file diff --git a/db/schema_migrations/20251111050850 b/db/schema_migrations/20251111050850 deleted file mode 100644 index a69ee04e1b16b9..00000000000000 --- a/db/schema_migrations/20251111050850 +++ /dev/null @@ -1 +0,0 @@ -ca0f87aa98ef4f199e69cdbca7398e0738b4357e821b6305406c842cb0e05909 \ No newline at end of file diff --git a/db/schema_migrations/20251130050757 b/db/schema_migrations/20251130050757 new file mode 100644 index 00000000000000..c70be6c61dd1ae --- /dev/null +++ b/db/schema_migrations/20251130050757 @@ -0,0 +1 @@ +25e82f60a32baf8a0db034e01fb3bc8e289414981dc79221498b8ec2f23e03dd \ No newline at end of file diff --git a/db/schema_migrations/20251130050850 b/db/schema_migrations/20251130050850 new file mode 100644 index 00000000000000..18d86923a2c51f --- /dev/null +++ b/db/schema_migrations/20251130050850 @@ -0,0 +1 @@ +b0a2147466316437d66cd6ba421c55228ed438320a2e2cf592627008ac559dcc \ No newline at end of file -- GitLab From f2da804715b0d169d76e5f8fdc4dd072847a9fe5 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 1 Dec 2025 11:05:26 +1100 Subject: [PATCH 3/5] include required partition helper module --- ...50850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb index 044164bc4f732c..789a2b034e6f6f 100644 --- a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb +++ b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + disable_ddl_transaction! milestone '18.7' -- GitLab From 6b918921038b732d1ccc2f53e0c686c7f5d685a4 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 4 Dec 2025 09:59:24 +1100 Subject: [PATCH 4/5] Address comments --- ...oreign_keys_on_merge_requests_stage_two.rb | 22 ++++++++++----- .../foreign_key_helpers.rb | 28 +++++++++++-------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb index 789a2b034e6f6f..8a3ca0749bfb79 100644 --- a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb +++ b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb @@ -15,7 +15,8 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_table: :projects, target_column: :id, on_delete: :cascade, - name: :fk_a6963e8447 + name: :fk_a6963e8447, + reverse_lock_order: true }, { source_table: :merge_requests, @@ -23,7 +24,8 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_table: :merge_request_diffs, target_column: :id, on_delete: :nullify, - name: :fk_06067f5644 + name: :fk_06067f5644, + reverse_lock_order: false } ].freeze PARTITIONED_FOREIGN_KEYS = [ @@ -33,7 +35,8 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_table: :merge_requests, target_column: [:target_project_id_convert_to_bigint, :iid], on_delete: :cascade, - name: :fk_generated_ref_commits_merge_request_id + name: :fk_generated_ref_commits_merge_request_id, + reverse_lock_order: false } ].freeze @@ -56,7 +59,7 @@ def up name: tmp_name(fk[:name]), on_delete: fk[:on_delete], validate: false, - reverse_lock_order: true + reverse_lock_order: fk[:reverse_lock_order] ) prepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) @@ -70,8 +73,11 @@ def up target_column: fk[:target_column], name: tmp_name(fk[:name]), on_delete: fk[:on_delete], - reverse_lock_order: true + validate: false, + reverse_lock_order: fk[:reverse_lock_order] ) + + prepare_partitioned_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) end end @@ -81,8 +87,10 @@ def down fk[:source_table], fk[:target_table], name: tmp_name(fk[:name]), - reverse_lock_order: true + reverse_lock_order: fk[:reverse_lock_order] ) + + unprepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) end FOREIGN_KEYS.each do |fk| @@ -90,7 +98,7 @@ def down fk[:source_table], fk[:target_table], name: tmp_name(fk[:name]), - reverse_lock_order: true + reverse_lock_order: fk[:reverse_lock_order] ) unprepare_async_foreign_key_validation fk[:source_table], fk[:column], name: tmp_name(fk[:name]) diff --git a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb index 8a9ee717c6ca79..2c14c331d5a539 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb @@ -20,13 +20,12 @@ module ForeignKeyHelpers # - Also, PostgreSQL will currently ignore NOT VALID constraints on partitions # when adding a valid FK to the partitioned table, so they have to # also be validated before we can add the final FK. + # # Solution: # - Add the foreign key first to each partition by using # add_concurrent_foreign_key and validating it # - Once all partitions have a foreign key, add it also to the partitioned # table (there will be no need for a validation at that level) - # For those reasons, this method does not include an option to delay the - # validation, we have to force validate: true. # # source - The source (partitioned) table containing the foreign key. # target - The target table the key points to. @@ -36,16 +35,21 @@ module ForeignKeyHelpers # on_update - The action to perform when associated data is updated, # no default value is set. # name - The name of the foreign key. - # validate - Flag that controls whether the new foreign key will be - # validated after creation and if it will be added on the parent table. - # If the flag is not set, the constraint will only be enforced for new data - # in the existing partitions. The helper will need to be called again - # with the flag set to `true` to add the foreign key on the parent table - # after validating it on all partitions. - # `validate: false` should be paired with `prepare_partitioned_async_foreign_key_validation` - # reverse_lock_order - Flag that controls whether we should attempt to acquire locks in the reverse - # order of the ALTER TABLE. This can be useful in situations where the foreign - # key creation could deadlock with another process. + # validate - Flag that controls whether the new foreign key will be validated + # after creation and added to the parent table. + # When true (default): The FK is validated on all partitions and then + # added to the parent partitioned table. + # When false: The FK is added to existing partitions as NOT VALID and + # will only be enforced for new data. The FK will NOT be added to the + # parent table. To complete the process, you must: + # 1. Call `prepare_partitioned_async_foreign_key_validation` to schedule + # async validation on all partitions + # 2. Call this helper again with `validate: true` after validation completes + # to add the FK to the parent table + # reverse_lock_order - Flag that controls whether we should attempt to acquire locks + # in the reverse order of the ALTER TABLE. This can be useful in + # situations where the foreign key creation could deadlock with + # another process. # def add_concurrent_partitioned_foreign_key(source, target, column:, **options) assert_not_in_transaction_block(scope: ERROR_SCOPE) -- GitLab From 58b5d50dc58fecaa5491af13dcc9798961d7f1fd Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 4 Dec 2025 14:20:06 +1100 Subject: [PATCH 5/5] Update lock order --- ...850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb index 8a3ca0749bfb79..15d0f1d3573179 100644 --- a/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb +++ b/db/post_migrate/20251130050850_add_bigint_foreign_keys_on_merge_requests_stage_two.rb @@ -25,7 +25,7 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_column: :id, on_delete: :nullify, name: :fk_06067f5644, - reverse_lock_order: false + reverse_lock_order: true } ].freeze PARTITIONED_FOREIGN_KEYS = [ @@ -36,7 +36,7 @@ class AddBigintForeignKeysOnMergeRequestsStageTwo < Gitlab::Database::Migration[ target_column: [:target_project_id_convert_to_bigint, :iid], on_delete: :cascade, name: :fk_generated_ref_commits_merge_request_id, - reverse_lock_order: false + reverse_lock_order: true } ].freeze -- GitLab