diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index fe92ebc768a494243f1907f756afe0473b90b83d..0d5e3c233f626cd23a4887dc356c20d4d99f52a7 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -381,6 +381,12 @@ result in a `Key is still referenced from table ...` error and updating the partition column on the source table would raise a `Key is not present in table ...` error. +This migration can be automatically generated using: + +```shell +./scripts/partitioning/generate-fk --source source_table_name --target target_table_name +``` + ### Step 5 - Swap primary key Swap the primary key including the partitioning key column. This can be done only after diff --git a/lib/generators/gitlab/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb new file mode 100644 index 0000000000000000000000000000000000000000..e013188c75330ec017b819ffa22bcbec3680f431 --- /dev/null +++ b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record/migration' + +module Gitlab + module Partitioning + class ForeignKeysGenerator < Rails::Generators::Base + include ActiveRecord::Generators::Migration + + desc 'This generator creates the migrations needed for updating the foreign keys when partitioning the tables' + + source_root File.expand_path('templates', __dir__) + + class_option :target, type: :string, required: true, desc: 'Target table name' + class_option :source, type: :string, required: true, desc: 'Source table name' + class_option :partitioning_column, type: :string, default: :partition_id, + desc: 'The column that is used for partitioning' + class_option :database, type: :string, default: :ci, + desc: 'Database name connection' + + def create_fk_index_migration + migration_template( + '../templates/foreign_key_index.rb.template', + fk_index_file_name) + end + + def create_fk_definition_migration + migration_template( + '../templates/foreign_key_definition.rb.template', + fk_definition_file_name) + end + + def create_fk_validation_migration + migration_template( + '../templates/foreign_key_validation.rb.template', + fk_validation_file_name) + end + + def remove_old_fk_migration + migration_template( + '../templates/foreign_key_removal.rb.template', + fk_removal_file_name) + end + + private + + def fk_index_file_name + post_migration_file_path( + "add_fk_index_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_definition_file_name + post_migration_file_path( + "add_fk_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_validation_file_name + post_migration_file_path( + "validate_fk_on_#{source_table_name}_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_removal_file_name + post_migration_file_path( + "remove_fk_to_#{target_table_name}_#{source_table_name}_on_#{foreign_key_column}.rb") + end + + def post_migration_file_path(name) + File.join(db_migrate_path, name) + end + + def db_migrate_path + super.sub('migrate', 'post_migrate') + end + + def source_table_name + options[:source] + end + + def target_table_name + options[:target] + end + + def partitioning_column + options[:partitioning_column] + end + + def foreign_keys_candidates + connection + .foreign_keys(source_table_name) + .select { |fk| fk.to_table == target_table_name } + .reject { |fk| fk.name.end_with?('_p') } + end + + def fk_candidate + @fk_candidate ||= select_foreign_key + end + + def foreign_key_name + fk_candidate.name + end + + def partitioned_foreign_key_name + "#{foreign_key_name}_p" + end + + def foreign_key_column + fk_candidate.column + end + + def fk_on_delete_option + fk_candidate.on_delete + end + + def fk_target_column + fk_candidate.primary_key + end + + def connection + Gitlab::Database + .database_base_models + .fetch(options[:database]) + .connection + end + + def select_foreign_key + case foreign_keys_candidates.size + when 0 + raise Thor::InvocationError, "No FK found between #{source_table_name} and #{target_table_name}" + when 1 + foreign_keys_candidates.first + else + select_fk_from_user_input + end + end + + def select_fk_from_user_input + options = (0...foreign_keys_candidates.size).to_a.map(&:to_s) + + say "There are multiple FKs between #{source_table_name} and #{target_table_name}:" + foreign_keys_candidates.each.with_index do |fk, index| + say "\t#{index} : #{fk}" + end + + input = ask("Please select one:", limited_to: options, default: '0') + + foreign_keys_candidates.fetch(input.to_i) + end + end + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template new file mode 100644 index 0000000000000000000000000000000000000000..f2f9acea923ce7e22ecdb2d1c78604f357f0be6a --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= partitioned_foreign_key_name %> + PARTITION_COLUMN = :<%= partitioning_column %> + + 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: :<%= fk_on_delete_option %>, + 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/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template new file mode 100644 index 0000000000000000000000000000000000000000..4896d9313333fa69fafb7ea2c5b6130ec6e37097 --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + INDEX_NAME = :index_<%= source_table_name -%>_on_<%= partitioning_column -%>_<%= foreign_key_column %> + TABLE_NAME = :<%= source_table_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + add_concurrent_index(TABLE_NAME, COLUMNS, name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template new file mode 100644 index 0000000000000000000000000000000000000000..b4e881074ad93665ed377515eb93005b8deda270 --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= foreign_key_name %> + + 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_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: COLUMN, + target_column: TARGET_COLUMN, + validate: true, + reverse_lock_order: true, + on_delete: :<%= fk_on_delete_option %>, + name: FK_NAME + ) + end +end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template new file mode 100644 index 0000000000000000000000000000000000000000..bad7d17a51b958f4d2fb1c0f1e2c011eda4fca9b --- /dev/null +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + TABLE_NAME = :<%= source_table_name %> + FK_NAME = :<%= partitioned_foreign_key_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + validate_foreign_key(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + # no-op + end +end diff --git a/scripts/partitioning/generate-fk b/scripts/partitioning/generate-fk new file mode 100755 index 0000000000000000000000000000000000000000..4f2dac1d61e2cc07011a2f3e3d02374c25bafd6c --- /dev/null +++ b/scripts/partitioning/generate-fk @@ -0,0 +1,3 @@ +#!/bin/bash + +exec bundle exec rails generate gitlab:partitioning:foreign_keys "$@" diff --git a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7c7ca8207ffd3e640c5f9473c2473ca4ba7f359b --- /dev/null +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'active_support/testing/stream' + +RSpec.describe Gitlab::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, +feature_category: :continuous_integration do + include ActiveSupport::Testing::Stream + include MigrationsHelpers + + before do + ActiveRecord::Schema.define do + create_table :_test_tmp_builds, force: :cascade do |t| + t.integer :partition_id + t.index [:id, :partition_id], unique: true + end + + create_table :_test_tmp_metadata, force: :cascade do |t| + t.integer :partition_id + t.references :builds, foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } + end + end + end + + after do + FileUtils.rm_rf(destination_root) + + table(:schema_migrations).where(version: migrations.map(&:version)).delete_all + + active_record_base.connection.execute(<<~SQL) + DROP TABLE _test_tmp_metadata; + DROP TABLE _test_tmp_builds; + SQL + end + + let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } + + let(:generator_config) { { destination_root: destination_root } } + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', '_test_tmp_builds', '--database', 'main'] } + + context 'without foreign keys' do + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', 'projects', '--database', 'main'] } + + it 'does not generate migrations' do + output = capture(:stderr) { run_generator } + + expect(migrations).to be_empty + expect(output).to match(/No FK found between _test_tmp_metadata and projects/) + end + end + + context 'with one FK' do + it 'generates foreign key migrations' do + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndBuildsId + AddFkToTestTmpMetadataOnPartitionIdAndBuildsId + ValidateFkOnTestTmpMetadataPartitionIdAndBuildsId + RemoveFkToTestTmpBuildsTestTmpMetadataOnBuildsId + ]) + + schema_migrate_up! + + fks = Gitlab::Database::PostgresForeignKey + .by_referenced_table_identifier('public._test_tmp_builds') + .by_constrained_table_identifier('public._test_tmp_metadata') + + expect(fks.size).to eq(1) + + foreign_key = fks.first + + expect(foreign_key.name).to end_with('_p') + expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) + expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) + expect(foreign_key.on_delete_action).to eq('cascade') + expect(foreign_key.on_update_action).to eq('cascade') + + index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| + index.columns == %w[partition_id builds_id] + end + + expect(index).to be_present + end + end + + context 'with many FKs' do + before do + ActiveRecord::Schema.define do + add_reference :_test_tmp_metadata, :job, + foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } + end + end + + it 'generates migrations for the selected FK' do + expect(Thor::LineEditor) + .to receive(:readline) + .with('Please select one: [0, 1] (0) ', { default: '0', limited_to: %w[0 1] }) + .and_return('1') + + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndJobId + AddFkToTestTmpMetadataOnPartitionIdAndJobId + ValidateFkOnTestTmpMetadataPartitionIdAndJobId + RemoveFkToTestTmpBuildsTestTmpMetadataOnJobId + ]) + end + end + + def run_generator(args = generator_args, config = generator_config) + described_class.start(args, config) + end + + # We want to execute only the newly generated migrations + def migrations_paths + [File.join(destination_root, 'db', 'post_migrate')] + end + + # There is no need to migrate down before executing the tests because these + # migrations were not already executed and we don't need to run it after + # the tests because we're removing the tables. + def schema_migrate_down! + # no-op + end +end