diff options
Diffstat (limited to 'spec/lib/gitlab/database/migrations')
8 files changed, 379 insertions, 41 deletions
diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb index e64f5807385..b0caa21e01a 100644 --- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb @@ -3,16 +3,31 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do + let(:base_class) { ActiveRecord::Migration } + let(:model) do - ActiveRecord::Migration.new.extend(described_class) + base_class.new + .extend(described_class) + .extend(Gitlab::Database::Migrations::ReestablishedConnectionStack) end - shared_examples_for 'helpers that enqueue background migrations' do |worker_class, tracking_database| + shared_examples_for 'helpers that enqueue background migrations' do |worker_class, connection_class, tracking_database| before do allow(model).to receive(:tracking_database).and_return(tracking_database) + + # Due to lib/gitlab/database/load_balancing/configuration.rb:92 requiring RequestStore + # we cannot use stub_feature_flags(force_no_sharing_primary_model: true) + allow(connection_class.connection.load_balancer.configuration) + .to receive(:use_dedicated_connection?).and_return(true) + + allow(model).to receive(:connection).and_return(connection_class.connection) end describe '#queue_background_migration_jobs_by_range_at_intervals' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + context 'when the model has an ID column' do let!(:id1) { create(:user).id } let!(:id2) { create(:user).id } @@ -196,6 +211,34 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end.to raise_error(StandardError, /does not have an ID/) end end + + context 'when using Migration[2.0]' do + let(:base_class) { Class.new(Gitlab::Database::Migration[2.0]) } + + context 'when restriction is set to gitlab_shared' do + before do + base_class.restrict_gitlab_migration gitlab_schema: :gitlab_shared + end + + it 'does raise an exception' do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(ProjectAuthorization, 'FooJob', 10.seconds) + end.to raise_error /use `restrict_gitlab_migration:` " with `:gitlab_shared`/ + end + end + end + + context 'when within transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(true) + end + + it 'does raise an exception' do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(ProjectAuthorization, 'FooJob', 10.seconds) + end.to raise_error /The `#queue_background_migration_jobs_by_range_at_intervals` can not be run inside a transaction./ + end + end end describe '#requeue_background_migration_jobs_by_range_at_intervals' do @@ -205,6 +248,10 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do let!(:successful_job_1) { create(:background_migration_job, class_name: job_class_name, status: :succeeded, arguments: [5, 6]) } let!(:successful_job_2) { create(:background_migration_job, class_name: job_class_name, status: :succeeded, arguments: [7, 8]) } + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + around do |example| freeze_time do Sidekiq::Testing.fake! do @@ -219,6 +266,38 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do expect(subject).to eq(20.minutes) end + context 'when using Migration[2.0]' do + let(:base_class) { Class.new(Gitlab::Database::Migration[2.0]) } + + it 'does re-enqueue pending jobs' do + subject + + expect(worker_class.jobs).not_to be_empty + end + + context 'when restriction is set' do + before do + base_class.restrict_gitlab_migration gitlab_schema: :gitlab_main + end + + it 'does raise an exception' do + expect { subject } + .to raise_error /The `#requeue_background_migration_jobs_by_range_at_intervals` cannot use `restrict_gitlab_migration:`./ + end + end + end + + context 'when within transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(true) + end + + it 'does raise an exception' do + expect { subject } + .to raise_error /The `#requeue_background_migration_jobs_by_range_at_intervals` can not be run inside a transaction./ + end + end + context 'when nothing is queued' do subject { model.requeue_background_migration_jobs_by_range_at_intervals('FakeJob', 10.minutes) } @@ -290,7 +369,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end end - describe '#finalized_background_migration' do + describe '#finalize_background_migration' do let(:coordinator) { Gitlab::BackgroundMigration::JobCoordinator.new(worker_class) } let!(:tracked_pending_job) { create(:background_migration_job, class_name: job_class_name, status: :pending, arguments: [1]) } @@ -309,8 +388,8 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do allow(Gitlab::BackgroundMigration).to receive(:coordinator_for_database) .with(tracking_database).and_return(coordinator) - expect(coordinator).to receive(:migration_class_for) - .with(job_class_name).at_least(:once) { job_class } + allow(coordinator).to receive(:migration_class_for) + .with(job_class_name) { job_class } Sidekiq::Testing.disable! do worker_class.perform_async(job_class_name, [1, 2]) @@ -318,6 +397,8 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do worker_class.perform_in(10, job_class_name, [5, 6]) worker_class.perform_in(20, job_class_name, [7, 8]) end + + allow(model).to receive(:transaction_open?).and_return(false) end it_behaves_like 'finalized tracked background migration', worker_class do @@ -326,6 +407,52 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end end + context 'when within transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(true) + end + + it 'does raise an exception' do + expect { model.finalize_background_migration(job_class_name, delete_tracking_jobs: %w[pending succeeded]) } + .to raise_error /The `#finalize_background_migration` can not be run inside a transaction./ + end + end + + context 'when using Migration[2.0]' do + let(:base_class) { Class.new(Gitlab::Database::Migration[2.0]) } + + it_behaves_like 'finalized tracked background migration', worker_class do + before do + model.finalize_background_migration(job_class_name) + end + end + + context 'when restriction is set' do + before do + base_class.restrict_gitlab_migration gitlab_schema: :gitlab_main + end + + it 'does raise an exception' do + expect { model.finalize_background_migration(job_class_name, delete_tracking_jobs: %w[pending succeeded]) } + .to raise_error /The `#finalize_background_migration` cannot use `restrict_gitlab_migration:`./ + end + end + end + + context 'when running migration in reconfigured ActiveRecord::Base context' do + it_behaves_like 'reconfigures connection stack', tracking_database do + it 'does restore connection hierarchy' do + expect_next_instances_of(job_class, 1..) do |job| + expect(job).to receive(:perform) do + validate_connections! + end + end + + model.finalize_background_migration(job_class_name, delete_tracking_jobs: %w[pending succeeded]) + end + end + end + context 'when removing all tracked job records' do let!(:job_class) do Class.new do @@ -443,7 +570,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end context 'when the migration is running against the main database' do - it_behaves_like 'helpers that enqueue background migrations', BackgroundMigrationWorker, 'main' + it_behaves_like 'helpers that enqueue background migrations', BackgroundMigrationWorker, ActiveRecord::Base, 'main' end context 'when the migration is running against the ci database', if: Gitlab::Database.has_config?(:ci) do @@ -453,7 +580,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end end - it_behaves_like 'helpers that enqueue background migrations', BackgroundMigration::CiDatabaseWorker, 'ci' + it_behaves_like 'helpers that enqueue background migrations', BackgroundMigration::CiDatabaseWorker, Ci::ApplicationRecord, 'ci' end describe '#delete_job_tracking' do diff --git a/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb new file mode 100644 index 00000000000..34c83c42056 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::BaseBackgroundRunner, :freeze_time do + let(:result_dir) { Dir.mktmpdir } + + after do + FileUtils.rm_rf(result_dir) + end + + context 'subclassing' do + subject { described_class.new(result_dir: result_dir) } + + it 'requires that jobs_by_migration_name be implemented' do + expect { subject.jobs_by_migration_name }.to raise_error(NotImplementedError) + end + + it 'requires that run_job be implemented' do + expect { subject.run_job(nil) }.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb index f9347a174c4..d1a66036149 100644 --- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb @@ -163,4 +163,45 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d end end end + + describe '#finalize_batched_background_migration' do + let!(:batched_migration) { create(:batched_background_migration, job_class_name: 'MyClass', table_name: :projects, column_name: :id, job_arguments: []) } + + it 'finalizes the migration' do + allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| + expect(runner).to receive(:finalize).with('MyClass', :projects, :id, []) + end + + migration.finalize_batched_background_migration(job_class_name: 'MyClass', table_name: :projects, column_name: :id, job_arguments: []) + end + + context 'when the migration does not exist' do + it 'raises an exception' do + expect do + migration.finalize_batched_background_migration(job_class_name: 'MyJobClass', table_name: :projects, column_name: :id, job_arguments: []) + end.to raise_error(RuntimeError, 'Could not find batched background migration') + end + end + + context 'when uses a CI connection', :reestablished_active_record_base do + before do + skip_if_multiple_databases_not_setup + + ActiveRecord::Base.establish_connection(:ci) # rubocop:disable Database/EstablishConnection + end + + it 'raises an exception' do + ci_migration = create(:batched_background_migration, :active) + + expect do + migration.finalize_batched_background_migration( + job_class_name: ci_migration.job_class_name, + table_name: ci_migration.table_name, + column_name: ci_migration.column_name, + job_arguments: ci_migration.job_arguments + ) + end.to raise_error /is currently not supported when running in decomposed/ + end + end + end end diff --git a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb index 2515f0d4a06..66de25d65bb 100644 --- a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb @@ -43,6 +43,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do <<~SQL SELECT query, calls, total_time, max_time, mean_time, rows FROM pg_stat_statements + WHERE pg_get_userbyid(userid) = current_user ORDER BY total_time DESC SQL end diff --git a/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb b/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb new file mode 100644 index 00000000000..cfb308c63e4 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::ReestablishedConnectionStack do + let(:base_class) { ActiveRecord::Migration } + + let(:model) do + base_class.new + .extend(described_class) + end + + describe '#with_restored_connection_stack' do + Gitlab::Database.database_base_models.each do |db_config_name, _| + context db_config_name do + it_behaves_like "reconfigures connection stack", db_config_name do + it 'does restore connection hierarchy' do + model.with_restored_connection_stack do + validate_connections! + end + end + + primary_db_config = ActiveRecord::Base.configurations.primary?(db_config_name) + + it 'does reconfigure connection handler', unless: primary_db_config do + original_handler = ActiveRecord::Base.connection_handler + new_handler = nil + + model.with_restored_connection_stack do + new_handler = ActiveRecord::Base.connection_handler + + # establish connection + ApplicationRecord.connection.select_one("SELECT 1 FROM projects LIMIT 1") + Ci::ApplicationRecord.connection.select_one("SELECT 1 FROM ci_builds LIMIT 1") + end + + expect(new_handler).not_to eq(original_handler), "is reconnected" + expect(new_handler).not_to be_active_connections + expect(ActiveRecord::Base.connection_handler).to eq(original_handler), "is restored" + end + + it 'does keep original connection handler', if: primary_db_config do + original_handler = ActiveRecord::Base.connection_handler + new_handler = nil + + model.with_restored_connection_stack do + new_handler = ActiveRecord::Base.connection_handler + end + + expect(new_handler).to eq(original_handler) + expect(ActiveRecord::Base.connection_handler).to eq(original_handler) + end + end + end + end + end +end diff --git a/spec/lib/gitlab/database/migrations/runner_spec.rb b/spec/lib/gitlab/database/migrations/runner_spec.rb index 8b1ccf05eb1..e7f68e3e4a8 100644 --- a/spec/lib/gitlab/database/migrations/runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/runner_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::Runner do + include Database::MultipleDatabases + let(:result_dir) { Pathname.new(Dir.mktmpdir) } let(:migration_runs) { [] } # This list gets populated as the runner tries to run migrations @@ -136,4 +138,35 @@ RSpec.describe Gitlab::Database::Migrations::Runner do expect(runner.result_dir).to eq(described_class::BASE_RESULT_DIR.join( 'background_migrations')) end end + + describe '.batched_background_migrations' do + it 'is a TestBatchedBackgroundRunner' do + expect(described_class.batched_background_migrations(for_database: 'main')).to be_a(Gitlab::Database::Migrations::TestBatchedBackgroundRunner) + end + + context 'choosing the database to test against' do + it 'chooses the main database' do + runner = described_class.batched_background_migrations(for_database: 'main') + + chosen_connection_name = Gitlab::Database.db_config_name(runner.connection) + + expect(chosen_connection_name).to eq('main') + end + + it 'chooses the ci database' do + skip_if_multiple_databases_not_setup + + runner = described_class.batched_background_migrations(for_database: 'ci') + + chosen_connection_name = Gitlab::Database.db_config_name(runner.connection) + + expect(chosen_connection_name).to eq('ci') + end + + it 'throws an error with an invalid name' do + expect { described_class.batched_background_migrations(for_database: 'not_a_database') } + .to raise_error(/not a valid database name/) + end + end + end end diff --git a/spec/lib/gitlab/database/migrations/test_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/test_background_runner_spec.rb index 9407efad91f..a2fe91712c7 100644 --- a/spec/lib/gitlab/database/migrations/test_background_runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/test_background_runner_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::TestBackgroundRunner, :redis do + include Gitlab::Database::Migrations::ReestablishedConnectionStack include Gitlab::Database::Migrations::BackgroundMigrationHelpers + include Database::MigrationTestingHelpers # In order to test the interaction between queueing sidekiq jobs and seeing those jobs in queues, # we need to disable sidekiq's testing mode and actually send our jobs to redis @@ -12,6 +14,7 @@ RSpec.describe Gitlab::Database::Migrations::TestBackgroundRunner, :redis do end let(:result_dir) { Dir.mktmpdir } + let(:connection) { ApplicationRecord.connection } after do FileUtils.rm_rf(result_dir) @@ -41,40 +44,6 @@ RSpec.describe Gitlab::Database::Migrations::TestBackgroundRunner, :redis do end context 'running migrations', :freeze_time do - def define_background_migration(name) - klass = Class.new do - # Can't simply def perform here as we won't have access to the block, - # similarly can't define_method(:perform, &block) here as it would change the block receiver - define_method(:perform) { |*args| yield(*args) } - end - stub_const("Gitlab::BackgroundMigration::#{name}", klass) - klass - end - - def expect_migration_call_counts(migrations_to_calls) - migrations_to_calls.each do |migration, calls| - expect_next_instances_of(migration, calls) do |m| - expect(m).to receive(:perform).and_call_original - end - end - end - - def expect_recorded_migration_runs(migrations_to_runs) - migrations_to_runs.each do |migration, runs| - path = File.join(result_dir, migration.name.demodulize) - num_subdirs = Pathname(path).children.count(&:directory?) - expect(num_subdirs).to eq(runs) - end - end - - def expect_migration_runs(migrations_to_run_counts) - expect_migration_call_counts(migrations_to_run_counts) - - yield - - expect_recorded_migration_runs(migrations_to_run_counts) - end - it 'runs the migration class correctly' do calls = [] define_background_migration(migration_name) do |i| diff --git a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb new file mode 100644 index 00000000000..fbfff1268cc --- /dev/null +++ b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freeze_time do + include Gitlab::Database::MigrationHelpers + include Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers + include Database::MigrationTestingHelpers + + let(:result_dir) { Dir.mktmpdir } + + after do + FileUtils.rm_rf(result_dir) + end + + let(:connection) { ApplicationRecord.connection } + + let(:table_name) { "_test_column_copying"} + + before do + connection.execute(<<~SQL) + CREATE TABLE #{table_name} ( + id bigint primary key not null, + data bigint + ); + + insert into #{table_name} (id) select i from generate_series(1, 1000) g(i); + SQL + end + + context 'running a real background migration' do + it 'runs sampled jobs from the batched background migration' do + queue_batched_background_migration('CopyColumnUsingBackgroundMigrationJob', + table_name, :id, + :id, :data, + batch_size: 100, + job_interval: 5.minutes) # job_interval is skipped when testing + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 1.minute) + unmigrated_row_count = define_batchable_model(table_name).where('id != data').count + + expect(unmigrated_row_count).to eq(0) + end + end + + context 'with jobs to run' do + let(:migration_name) { 'TestBackgroundMigration' } + + before do + queue_batched_background_migration(migration_name, table_name, :id, job_interval: 5.minutes, batch_size: 100) + end + + it 'samples jobs' do + calls = [] + define_background_migration(migration_name) do |*args| + calls << args + end + + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 3.minutes) + + expect(calls.count).to eq(10) # 1000 rows / batch size 100 = 10 + end + + context 'with multiple jobs to run' do + it 'runs all jobs created within the last 48 hours' do + old_migration = define_background_migration(migration_name) + + travel 3.days + + new_migration = define_background_migration('NewMigration') { travel 1.second } + queue_batched_background_migration('NewMigration', table_name, :id, + job_interval: 5.minutes, + batch_size: 10, + sub_batch_size: 5) + + other_new_migration = define_background_migration('NewMigration2') { travel 2.seconds } + queue_batched_background_migration('NewMigration2', table_name, :id, + job_interval: 5.minutes, + batch_size: 10, + sub_batch_size: 5) + + expect_migration_runs(new_migration => 3, other_new_migration => 2, old_migration => 0) do + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 5.seconds) + end + end + end + end +end |