diff options
Diffstat (limited to 'spec/lib/gitlab/database')
30 files changed, 1306 insertions, 312 deletions
diff --git a/spec/lib/gitlab/database/background_migration_job_spec.rb b/spec/lib/gitlab/database/background_migration_job_spec.rb new file mode 100644 index 00000000000..40f47325be3 --- /dev/null +++ b/spec/lib/gitlab/database/background_migration_job_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::BackgroundMigrationJob do + it_behaves_like 'having unique enum values' + + describe '.for_migration_execution' do + let!(:job1) { create(:background_migration_job) } + let!(:job2) { create(:background_migration_job, arguments: ['hi', 2]) } + let!(:job3) { create(:background_migration_job, class_name: 'OtherJob', arguments: ['hi', 2]) } + + it 'returns jobs matching class_name and arguments' do + relation = described_class.for_migration_execution('TestJob', ['hi', 2]) + + expect(relation.count).to eq(1) + expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: ['hi', 2]) + end + + it 'normalizes class names by removing leading ::' do + relation = described_class.for_migration_execution('::TestJob', ['hi', 2]) + + expect(relation.count).to eq(1) + expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: ['hi', 2]) + end + end + + describe '.for_partitioning_migration' do + let!(:job1) { create(:background_migration_job, arguments: [1, 100, 'other_table']) } + let!(:job2) { create(:background_migration_job, arguments: [1, 100, 'audit_events']) } + let!(:job3) { create(:background_migration_job, class_name: 'OtherJob', arguments: [1, 100, 'audit_events']) } + + it 'returns jobs matching class_name and the table_name job argument' do + relation = described_class.for_partitioning_migration('TestJob', 'audit_events') + + expect(relation.count).to eq(1) + expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: [1, 100, 'audit_events']) + end + + it 'normalizes class names by removing leading ::' do + relation = described_class.for_partitioning_migration('::TestJob', 'audit_events') + + expect(relation.count).to eq(1) + expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: [1, 100, 'audit_events']) + end + end + + describe '.mark_all_as_succeeded' do + let!(:job1) { create(:background_migration_job, arguments: [1, 100]) } + let!(:job2) { create(:background_migration_job, arguments: [1, 100]) } + let!(:job3) { create(:background_migration_job, arguments: [101, 200]) } + let!(:job4) { create(:background_migration_job, class_name: 'OtherJob', arguments: [1, 100]) } + + it 'marks all matching jobs as succeeded' do + expect { described_class.mark_all_as_succeeded('TestJob', [1, 100]) } + .to change { described_class.succeeded.count }.from(0).to(2) + + expect(job1.reload).to be_succeeded + expect(job2.reload).to be_succeeded + expect(job3.reload).to be_pending + expect(job4.reload).to be_pending + end + + it 'normalizes class_names by removing leading ::' do + expect { described_class.mark_all_as_succeeded('::TestJob', [1, 100]) } + .to change { described_class.succeeded.count }.from(0).to(2) + + expect(job1.reload).to be_succeeded + expect(job2.reload).to be_succeeded + expect(job3.reload).to be_pending + expect(job4.reload).to be_pending + end + + context 'when previous matching jobs have already succeeded' do + let(:initial_time) { Time.now.round } + let!(:job1) { create(:background_migration_job, :succeeded, created_at: initial_time, updated_at: initial_time) } + + it 'does not update non-pending jobs' do + Timecop.freeze(initial_time + 1.day) do + expect { described_class.mark_all_as_succeeded('TestJob', [1, 100]) } + .to change { described_class.succeeded.count }.from(1).to(2) + end + + expect(job1.reload.updated_at).to eq(initial_time) + expect(job2.reload).to be_succeeded + expect(job3.reload).to be_pending + expect(job4.reload).to be_pending + end + end + end + + describe '#class_name=' do + context 'when the class_name is given without the leading ::' do + it 'sets the class_name to the given value' do + job = described_class.new(class_name: 'TestJob') + + expect(job.class_name).to eq('TestJob') + end + end + + context 'when the class_name is given with the leading ::' do + it 'removes the leading :: when setting the class_name' do + job = described_class.new(class_name: '::TestJob') + + expect(job.class_name).to eq('TestJob') + end + end + + context 'when the value is nil' do + it 'sets the class_name to nil' do + job = described_class.new(class_name: nil) + + expect(job.class_name).to be_nil + end + end + + context 'when the values is blank' do + it 'sets the class_name to the given value' do + job = described_class.new(class_name: '') + + expect(job.class_name).to eq('') + end + end + end +end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index e7cb53f2dbd..656501dbf56 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::BatchCount do +RSpec.describe Gitlab::Database::BatchCount do let_it_be(:fallback) { ::Gitlab::Database::BatchCounter::FALLBACK } let_it_be(:small_batch_size) { ::Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE - 1 } let(:model) { Issue } diff --git a/spec/lib/gitlab/database/connection_timer_spec.rb b/spec/lib/gitlab/database/connection_timer_spec.rb index c9e9d770343..2b6746bae07 100644 --- a/spec/lib/gitlab/database/connection_timer_spec.rb +++ b/spec/lib/gitlab/database/connection_timer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::ConnectionTimer do +RSpec.describe Gitlab::Database::ConnectionTimer do let(:current_clock_value) { 1234.56 } before do diff --git a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb index 111833a506a..390620379d6 100644 --- a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::Count::ExactCountStrategy do +RSpec.describe Gitlab::Database::Count::ExactCountStrategy do before do create_list(:project, 3) create(:identity) diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb index 08032d19d14..324ed498abc 100644 --- a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::Count::ReltuplesCountStrategy do +RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do before do create_list(:project, 3) create(:identity) diff --git a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb index 0c480709c22..e488bf5ee4c 100644 --- a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::Count::TablesampleCountStrategy do +RSpec.describe Gitlab::Database::Count::TablesampleCountStrategy do before do create_list(:project, 3) create(:identity) diff --git a/spec/lib/gitlab/database/count_spec.rb b/spec/lib/gitlab/database/count_spec.rb index 2469ce482e7..d65413c2a00 100644 --- a/spec/lib/gitlab/database/count_spec.rb +++ b/spec/lib/gitlab/database/count_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::Count do +RSpec.describe Gitlab::Database::Count do before do create_list(:project, 3) create(:identity) diff --git a/spec/lib/gitlab/database/custom_structure_spec.rb b/spec/lib/gitlab/database/custom_structure_spec.rb index f03b5ed0a7f..beda9df3684 100644 --- a/spec/lib/gitlab/database/custom_structure_spec.rb +++ b/spec/lib/gitlab/database/custom_structure_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::CustomStructure do +RSpec.describe Gitlab::Database::CustomStructure do let_it_be(:structure) { described_class.new } let_it_be(:filepath) { Rails.root.join(described_class::CUSTOM_DUMP_FILE) } let_it_be(:file_header) do diff --git a/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb b/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb new file mode 100644 index 00000000000..23ad621d0ee --- /dev/null +++ b/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::DynamicModelHelpers do + describe '#define_batchable_model' do + subject { including_class.new.define_batchable_model(table_name) } + + let(:including_class) { Class.new.include(described_class) } + let(:table_name) { 'projects' } + + it 'is an ActiveRecord model' do + expect(subject.ancestors).to include(ActiveRecord::Base) + end + + it 'includes EachBatch' do + expect(subject.included_modules).to include(EachBatch) + end + + it 'has the correct table name' do + expect(subject.table_name).to eq(table_name) + end + + it 'has the inheritance type column disable' do + expect(subject.inheritance_column).to eq('_type_disabled') + end + end +end diff --git a/spec/lib/gitlab/database/grant_spec.rb b/spec/lib/gitlab/database/grant_spec.rb index 02697eb2a16..23aec85d1d0 100644 --- a/spec/lib/gitlab/database/grant_spec.rb +++ b/spec/lib/gitlab/database/grant_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::Grant do +RSpec.describe Gitlab::Database::Grant do describe '.create_and_execute_trigger' do it 'returns true when the user can create and execute a trigger' do # We assume the DB/user is set up correctly so that triggers can be diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index bed444ee7c7..48e1c97e97f 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::MigrationHelpers do +RSpec.describe Gitlab::Database::MigrationHelpers do let(:model) do ActiveRecord::Migration.new.extend(described_class) end @@ -178,6 +178,19 @@ describe Gitlab::Database::MigrationHelpers do model.remove_concurrent_index_by_name(:users, "index_x_by_y") end + + it 'removes the index with keyword arguments' do + expect(model).to receive(:remove_index) + .with(:users, { algorithm: :concurrently, name: "index_x_by_y" }) + + model.remove_concurrent_index_by_name(:users, name: "index_x_by_y") + end + + it 'raises an error if the index is blank' do + expect do + model.remove_concurrent_index_by_name(:users, wrong_key: "index_x_by_y") + end.to raise_error 'remove_concurrent_index_by_name must get an index name as the second argument' + end end end end @@ -690,12 +703,28 @@ describe Gitlab::Database::MigrationHelpers do model.rename_column_concurrently(:users, :old, :new) end + context 'with existing records and type casting' do + let(:trigger_name) { model.rename_trigger_name(:users, :id, :new) } + let(:user) { create(:user) } + + it 'copies the value to the new column using the type_cast_function', :aggregate_failures do + expect(model).to receive(:copy_indexes).with(:users, :id, :new) + expect(model).to receive(:add_not_null_constraint).with(:users, :new) + expect(model).to receive(:execute).with("UPDATE \"users\" SET \"new\" = cast_to_jsonb_with_default(\"users\".\"id\") WHERE \"users\".\"id\" >= #{user.id}") + expect(model).to receive(:execute).with("DROP TRIGGER IF EXISTS #{trigger_name}\nON \"users\"\n") + expect(model).to receive(:execute).with("CREATE TRIGGER #{trigger_name}\nBEFORE INSERT OR UPDATE\nON \"users\"\nFOR EACH ROW\nEXECUTE PROCEDURE #{trigger_name}()\n") + expect(model).to receive(:execute).with("CREATE OR REPLACE FUNCTION #{trigger_name}()\nRETURNS trigger AS\n$BODY$\nBEGIN\n NEW.\"new\" := NEW.\"id\";\n RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n") + + model.rename_column_concurrently(:users, :id, :new, type_cast_function: 'cast_to_jsonb_with_default') + end + end + it 'passes the batch_column_name' do expect(model).to receive(:column_exists?).with(:users, :other_batch_column).and_return(true) expect(model).to receive(:check_trigger_permissions!).and_return(true) expect(model).to receive(:create_column_from).with( - :users, :old, :new, type: nil, batch_column_name: :other_batch_column + :users, :old, :new, type: nil, batch_column_name: :other_batch_column, type_cast_function: nil ).and_return(true) expect(model).to receive(:install_rename_triggers).and_return(true) @@ -703,6 +732,14 @@ describe Gitlab::Database::MigrationHelpers do model.rename_column_concurrently(:users, :old, :new, batch_column_name: :other_batch_column) end + it 'passes the type_cast_function' do + expect(model).to receive(:create_column_from).with( + :users, :old, :new, type: nil, batch_column_name: :id, type_cast_function: 'JSON' + ).and_return(true) + + model.rename_column_concurrently(:users, :old, :new, type_cast_function: 'JSON') + end + it 'raises an error with invalid batch_column_name' do expect do model.rename_column_concurrently(:users, :old, :new, batch_column_name: :invalid) @@ -866,10 +903,19 @@ describe Gitlab::Database::MigrationHelpers do describe '#change_column_type_concurrently' do it 'changes the column type' do expect(model).to receive(:rename_column_concurrently) - .with('users', 'username', 'username_for_type_change', type: :text) + .with('users', 'username', 'username_for_type_change', type: :text, type_cast_function: nil) model.change_column_type_concurrently('users', 'username', :text) end + + context 'with type cast' do + it 'changes the column type with casting the value to the new type' do + expect(model).to receive(:rename_column_concurrently) + .with('users', 'username', 'username_for_type_change', type: :text, type_cast_function: 'JSON') + + model.change_column_type_concurrently('users', 'username', :text, type_cast_function: 'JSON') + end + end end describe '#cleanup_concurrent_column_type_change' do @@ -1215,166 +1261,6 @@ describe Gitlab::Database::MigrationHelpers do end end - describe '#bulk_queue_background_migration_jobs_by_range' do - context 'when the model has an ID column' do - let!(:id1) { create(:user).id } - let!(:id2) { create(:user).id } - let!(:id3) { create(:user).id } - - before do - User.class_eval do - include EachBatch - end - end - - context 'with enough rows to bulk queue jobs more than once' do - before do - stub_const('Gitlab::Database::MigrationHelpers::BACKGROUND_MIGRATION_JOB_BUFFER_SIZE', 1) - end - - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) - expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) - end - end - - it 'queues jobs in groups of buffer size 1' do - expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]]]) - expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id3, id3]]]) - - model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) - end - end - - context 'with not enough rows to bulk queue jobs more than once' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) - expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) - end - end - - it 'queues jobs in bulk all at once (big buffer size)' do - expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]], - ['FooJob', [id3, id3]]]) - - model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) - end - end - - context 'without specifying batch_size' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob') - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3]]) - end - end - end - end - - context "when the model doesn't have an ID column" do - it 'raises error (for now)' do - expect do - model.bulk_queue_background_migration_jobs_by_range(ProjectAuthorization, 'FooJob') - end.to raise_error(StandardError, /does not have an ID/) - end - end - end - - describe '#queue_background_migration_jobs_by_range_at_intervals' do - context 'when the model has an ID column' do - let!(:id1) { create(:user).id } - let!(:id2) { create(:user).id } - let!(:id3) { create(:user).id } - - around do |example| - Timecop.freeze { example.run } - end - - before do - User.class_eval do - include EachBatch - end - end - - it 'returns the final expected delay' do - Sidekiq::Testing.fake! do - final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2) - - expect(final_delay.to_f).to eq(20.minutes.to_f) - end - end - - it 'returns zero when nothing gets queued' do - Sidekiq::Testing.fake! do - final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User.none, 'FooJob', 10.minutes) - - expect(final_delay).to eq(0) - end - end - - context 'with batch_size option' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) - expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) - expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.minutes.from_now.to_f) - end - end - end - - context 'without batch_size option' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) - end - end - end - - context 'with other_job_arguments option' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2]) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) - end - end - end - - context 'with initial_delay option' do - it 'queues jobs correctly' do - Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2], initial_delay: 10.minutes) - - expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(20.minutes.from_now.to_f) - end - end - end - end - - context "when the model doesn't have an ID column" do - it 'raises error (for now)' do - expect do - model.queue_background_migration_jobs_by_range_at_intervals(ProjectAuthorization, 'FooJob', 10.seconds) - end.to raise_error(StandardError, /does not have an ID/) - end - end - end - describe '#change_column_type_using_background_migration' do let!(:issue) { create(:issue, :closed, closed_at: Time.zone.now) } @@ -1485,26 +1371,6 @@ describe Gitlab::Database::MigrationHelpers do end end - describe '#perform_background_migration_inline?' do - it 'returns true in a test environment' do - stub_rails_env('test') - - expect(model.perform_background_migration_inline?).to eq(true) - end - - it 'returns true in a development environment' do - stub_rails_env('development') - - expect(model.perform_background_migration_inline?).to eq(true) - end - - it 'returns false in a production environment' do - stub_rails_env('production') - - expect(model.perform_background_migration_inline?).to eq(false) - end - end - describe '#index_exists_by_name?' do it 'returns true if an index exists' do ActiveRecord::Base.connection.execute( @@ -1973,62 +1839,6 @@ describe Gitlab::Database::MigrationHelpers do end end - describe '#migrate_async' do - it 'calls BackgroundMigrationWorker.perform_async' do - expect(BackgroundMigrationWorker).to receive(:perform_async).with("Class", "hello", "world") - - model.migrate_async("Class", "hello", "world") - end - - it 'pushes a context with the current class name as caller_id' do - expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) - - model.migrate_async('Class', 'hello', 'world') - end - end - - describe '#migrate_in' do - it 'calls BackgroundMigrationWorker.perform_in' do - expect(BackgroundMigrationWorker).to receive(:perform_in).with(10.minutes, 'Class', 'Hello', 'World') - - model.migrate_in(10.minutes, 'Class', 'Hello', 'World') - end - - it 'pushes a context with the current class name as caller_id' do - expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) - - model.migrate_in(10.minutes, 'Class', 'Hello', 'World') - end - end - - describe '#bulk_migrate_async' do - it 'calls BackgroundMigrationWorker.bulk_perform_async' do - expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([%w(Class hello world)]) - - model.bulk_migrate_async([%w(Class hello world)]) - end - - it 'pushes a context with the current class name as caller_id' do - expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) - - model.bulk_migrate_async([%w(Class hello world)]) - end - end - - describe '#bulk_migrate_in' do - it 'calls BackgroundMigrationWorker.bulk_perform_in_' do - expect(BackgroundMigrationWorker).to receive(:bulk_perform_in).with(10.minutes, [%w(Class hello world)]) - - model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) - end - - it 'pushes a context with the current class name as caller_id' do - expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) - - model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) - end - end - describe '#check_constraint_name' do it 'returns a valid constraint name' do name = model.check_constraint_name(:this_is_a_very_long_table_name, diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb new file mode 100644 index 00000000000..042ac498373 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb @@ -0,0 +1,276 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + describe '#bulk_queue_background_migration_jobs_by_range' do + context 'when the model has an ID column' do + let!(:id1) { create(:user).id } + let!(:id2) { create(:user).id } + let!(:id3) { create(:user).id } + + before do + User.class_eval do + include EachBatch + end + end + + context 'with enough rows to bulk queue jobs more than once' do + before do + stub_const('Gitlab::Database::Migrations::BackgroundMigrationHelpers::BACKGROUND_MIGRATION_JOB_BUFFER_SIZE', 1) + end + + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) + end + end + + it 'queues jobs in groups of buffer size 1' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id3, id3]]]) + + model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) + end + end + + context 'with not enough rows to bulk queue jobs more than once' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) + end + end + + it 'queues jobs in bulk all at once (big buffer size)' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]], + ['FooJob', [id3, id3]]]) + + model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) + end + end + + context 'without specifying batch_size' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob') + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3]]) + end + end + end + end + + context "when the model doesn't have an ID column" do + it 'raises error (for now)' do + expect do + model.bulk_queue_background_migration_jobs_by_range(ProjectAuthorization, 'FooJob') + end.to raise_error(StandardError, /does not have an ID/) + end + end + end + + describe '#queue_background_migration_jobs_by_range_at_intervals' do + context 'when the model has an ID column' do + let!(:id1) { create(:user).id } + let!(:id2) { create(:user).id } + let!(:id3) { create(:user).id } + + around do |example| + Timecop.freeze { example.run } + end + + before do + User.class_eval do + include EachBatch + end + end + + it 'returns the final expected delay' do + Sidekiq::Testing.fake! do + final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2) + + expect(final_delay.to_f).to eq(20.minutes.to_f) + end + end + + it 'returns zero when nothing gets queued' do + Sidekiq::Testing.fake! do + final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User.none, 'FooJob', 10.minutes) + + expect(final_delay).to eq(0) + end + end + + context 'with batch_size option' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) + expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.minutes.from_now.to_f) + end + end + end + + context 'without batch_size option' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3]]) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) + end + end + end + + context 'with other_job_arguments option' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2]) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]]) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) + end + end + end + + context 'with initial_delay option' do + it 'queues jobs correctly' do + Sidekiq::Testing.fake! do + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2], initial_delay: 10.minutes) + + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3, 1, 2]]) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(20.minutes.from_now.to_f) + end + end + end + + context 'with track_jobs option' do + it 'creates a record for each job in the database' do + Sidekiq::Testing.fake! do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(User, '::FooJob', 10.minutes, + other_job_arguments: [1, 2], track_jobs: true) + end.to change { Gitlab::Database::BackgroundMigrationJob.count }.from(0).to(1) + + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + + tracked_job = Gitlab::Database::BackgroundMigrationJob.first + + expect(tracked_job.class_name).to eq('FooJob') + expect(tracked_job.arguments).to eq([id1, id3, 1, 2]) + expect(tracked_job).to be_pending + end + end + end + + context 'without track_jobs option' do + it 'does not create records in the database' do + Sidekiq::Testing.fake! do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, other_job_arguments: [1, 2]) + end.not_to change { Gitlab::Database::BackgroundMigrationJob.count } + + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + end + end + end + end + + context "when the model doesn't have an ID column" do + it 'raises error (for now)' do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(ProjectAuthorization, 'FooJob', 10.seconds) + end.to raise_error(StandardError, /does not have an ID/) + end + end + end + + describe '#perform_background_migration_inline?' do + it 'returns true in a test environment' do + stub_rails_env('test') + + expect(model.perform_background_migration_inline?).to eq(true) + end + + it 'returns true in a development environment' do + stub_rails_env('development') + + expect(model.perform_background_migration_inline?).to eq(true) + end + + it 'returns false in a production environment' do + stub_rails_env('production') + + expect(model.perform_background_migration_inline?).to eq(false) + end + end + + describe '#migrate_async' do + it 'calls BackgroundMigrationWorker.perform_async' do + expect(BackgroundMigrationWorker).to receive(:perform_async).with("Class", "hello", "world") + + model.migrate_async("Class", "hello", "world") + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.migrate_async('Class', 'hello', 'world') + end + end + + describe '#migrate_in' do + it 'calls BackgroundMigrationWorker.perform_in' do + expect(BackgroundMigrationWorker).to receive(:perform_in).with(10.minutes, 'Class', 'Hello', 'World') + + model.migrate_in(10.minutes, 'Class', 'Hello', 'World') + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.migrate_in(10.minutes, 'Class', 'Hello', 'World') + end + end + + describe '#bulk_migrate_async' do + it 'calls BackgroundMigrationWorker.bulk_perform_async' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([%w(Class hello world)]) + + model.bulk_migrate_async([%w(Class hello world)]) + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.bulk_migrate_async([%w(Class hello world)]) + end + end + + describe '#bulk_migrate_in' do + it 'calls BackgroundMigrationWorker.bulk_perform_in_' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_in).with(10.minutes, [%w(Class hello world)]) + + model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) + end + end +end diff --git a/spec/lib/gitlab/database/multi_threaded_migration_spec.rb b/spec/lib/gitlab/database/multi_threaded_migration_spec.rb index 53c001fbc1b..78dd9e88064 100644 --- a/spec/lib/gitlab/database/multi_threaded_migration_spec.rb +++ b/spec/lib/gitlab/database/multi_threaded_migration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::MultiThreadedMigration do +RSpec.describe Gitlab::Database::MultiThreadedMigration do let(:migration) do Class.new { include Gitlab::Database::MultiThreadedMigration }.new end diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index dee1d7df1a9..034bf966db7 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::ObsoleteIgnoredColumns do +RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do before do stub_const('Testing', Module.new) stub_const('Testing::MyBase', Class.new(ActiveRecord::Base)) diff --git a/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb new file mode 100644 index 00000000000..334cac653cf --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::MonthlyStrategy do + describe '#current_partitions' do + subject { described_class.new(model, partitioning_key).current_partitions } + + let(:model) { double('model', table_name: table_name) } + let(:partitioning_key) { double } + let(:table_name) { :partitioned_test } + + before do + ActiveRecord::Base.connection.execute(<<~SQL) + CREATE TABLE #{table_name} + (id serial not null, created_at timestamptz not null, PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); + + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.partitioned_test_000000 + PARTITION OF #{table_name} + FOR VALUES FROM (MINVALUE) TO ('2020-05-01'); + + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.partitioned_test_202005 + PARTITION OF #{table_name} + FOR VALUES FROM ('2020-05-01') TO ('2020-06-01'); + SQL + end + + it 'detects both partitions' do + expect(subject).to eq([ + Gitlab::Database::Partitioning::TimePartition.new(table_name, nil, '2020-05-01', partition_name: 'partitioned_test_000000'), + Gitlab::Database::Partitioning::TimePartition.new(table_name, '2020-05-01', '2020-06-01', partition_name: 'partitioned_test_202005') + ]) + end + end + + describe '#missing_partitions' do + subject { described_class.new(model, partitioning_key).missing_partitions } + + let(:model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'partitioned_test' + self.primary_key = :id + end + end + + let(:partitioning_key) { :created_at } + + around do |example| + Timecop.freeze(Date.parse('2020-08-22')) { example.run } + end + + context 'with existing partitions' do + before do + ActiveRecord::Base.connection.execute(<<~SQL) + CREATE TABLE #{model.table_name} + (id serial not null, created_at timestamptz not null, PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); + + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.partitioned_test_000000 + PARTITION OF #{model.table_name} + FOR VALUES FROM (MINVALUE) TO ('2020-05-01'); + + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.partitioned_test_202006 + PARTITION OF #{model.table_name} + FOR VALUES FROM ('2020-06-01') TO ('2020-07-01'); + SQL + + # Insert some data, it doesn't make a difference + model.create!(created_at: Date.parse('2020-04-20')) + model.create!(created_at: Date.parse('2020-06-15')) + end + + it 'detects the gap and the missing partition in May 2020' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-05-01', '2020-06-01')) + end + + it 'detects the missing partitions at the end of the range and expects a partition for July 2020' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-07-01', '2020-08-01')) + end + + it 'detects the missing partitions at the end of the range and expects a partition for August 2020' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-08-01', '2020-09-01')) + end + + it 'creates partitions 6 months out from now (Sep 2020 through Feb 2021)' do + expect(subject).to include( + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-09-01', '2020-10-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-10-01', '2020-11-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-11-01', '2020-12-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-12-01', '2021-01-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2021-01-01', '2021-02-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2021-02-01', '2021-03-01') + ) + end + + it 'detects all missing partitions' do + expect(subject.size).to eq(9) + end + end + + context 'without existing partitions' do + before do + ActiveRecord::Base.connection.execute(<<~SQL) + CREATE TABLE #{model.table_name} + (id serial not null, created_at timestamptz not null, PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); + SQL + end + + it 'detects the missing catch-all partition at the beginning' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, nil, '2020-08-01')) + end + + it 'detects the missing partition for today and expects a partition for August 2020' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-08-01', '2020-09-01')) + end + + it 'creates partitions 6 months out from now (Sep 2020 through Feb 2021' do + expect(subject).to include( + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-09-01', '2020-10-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-10-01', '2020-11-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-11-01', '2020-12-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2020-12-01', '2021-01-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2021-01-01', '2021-02-01'), + Gitlab::Database::Partitioning::TimePartition.new(model.table_name, '2021-02-01', '2021-03-01') + ) + end + + it 'detects all missing partitions' do + expect(subject.size).to eq(8) + end + end + + context 'with a regular partition but no catchall (MINVALUE, to) partition' do + before do + ActiveRecord::Base.connection.execute(<<~SQL) + CREATE TABLE #{model.table_name} + (id serial not null, created_at timestamptz not null, PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); + + CREATE TABLE #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.partitioned_test_202006 + PARTITION OF #{model.table_name} + FOR VALUES FROM ('2020-06-01') TO ('2020-07-01'); + SQL + end + + it 'detects a missing catch-all partition to add before the existing partition' do + expect(subject).to include(Gitlab::Database::Partitioning::TimePartition.new(model.table_name, nil, '2020-06-01')) + end + end + end +end diff --git a/spec/lib/gitlab/database/partitioning/partition_creator_spec.rb b/spec/lib/gitlab/database/partitioning/partition_creator_spec.rb new file mode 100644 index 00000000000..56399941662 --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/partition_creator_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::PartitionCreator do + include PartitioningHelpers + include ExclusiveLeaseHelpers + + describe '.register' do + let(:model) { double(partitioning_strategy: nil) } + + it 'remembers registered models' do + expect { described_class.register(model) }.to change { described_class.models }.to include(model) + end + end + + describe '#create_partitions (mocked)' do + subject { described_class.new(models).create_partitions } + + let(:models) { [model] } + let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table) } + let(:partitioning_strategy) { double(missing_partitions: partitions) } + let(:table) { "some_table" } + + before do + allow(ActiveRecord::Base.connection).to receive(:table_exists?).and_call_original + allow(ActiveRecord::Base.connection).to receive(:table_exists?).with(table).and_return(true) + allow(ActiveRecord::Base.connection).to receive(:execute).and_call_original + + stub_exclusive_lease(described_class::LEASE_KEY % table, timeout: described_class::LEASE_TIMEOUT) + end + + let(:partitions) do + [ + instance_double(Gitlab::Database::Partitioning::TimePartition, table: 'bar', partition_name: 'foo', to_sql: "SELECT 1"), + instance_double(Gitlab::Database::Partitioning::TimePartition, table: 'bar', partition_name: 'foo2', to_sql: "SELECT 2") + ] + end + + it 'creates the partition' do + expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.first.to_sql) + expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.second.to_sql) + + subject + end + + context 'error handling with 2 models' do + let(:models) do + [ + double(partitioning_strategy: strategy1, table_name: table), + double(partitioning_strategy: strategy2, table_name: table) + ] + end + + let(:strategy1) { double('strategy1', missing_partitions: nil) } + let(:strategy2) { double('strategy2', missing_partitions: partitions) } + + it 'still creates partitions for the second table' do + expect(strategy1).to receive(:missing_partitions).and_raise('this should never happen (tm)') + expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.first.to_sql) + expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.second.to_sql) + + subject + end + end + end + + describe '#create_partitions' do + subject { described_class.new([my_model]).create_partitions } + + let(:connection) { ActiveRecord::Base.connection } + let(:my_model) do + Class.new(ApplicationRecord) do + include PartitionedTable + + self.table_name = 'my_model_example_table' + + partitioned_by :created_at, strategy: :monthly + end + end + + before do + connection.execute(<<~SQL) + CREATE TABLE my_model_example_table + (id serial not null, created_at timestamptz not null, primary key (id, created_at)) + PARTITION BY RANGE (created_at); + SQL + end + + it 'creates partitions' do + expect { subject }.to change { find_partitions(my_model.table_name, schema: Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA).size }.from(0) + + subject + end + end +end diff --git a/spec/lib/gitlab/database/partitioning/time_partition_spec.rb b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb new file mode 100644 index 00000000000..700202d81c5 --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::TimePartition do + describe '.from_sql' do + subject { described_class.from_sql(table, partition_name, definition) } + + let(:table) { 'foo' } + let(:partition_name) { 'foo_bar' } + let(:definition) { 'FOR VALUES FROM (\'2020-04-01 00:00:00\') TO (\'2020-05-01 00:00:00\')' } + + it 'uses specified table name' do + expect(subject.table).to eq(table) + end + + it 'uses specified partition name' do + expect(subject.partition_name).to eq(partition_name) + end + + it 'parses start date' do + expect(subject.from).to eq(Date.parse('2020-04-01')) + end + + it 'parses end date' do + expect(subject.to).to eq(Date.parse('2020-05-01')) + end + + context 'with MINVALUE as a start date' do + let(:definition) { 'FOR VALUES FROM (MINVALUE) TO (\'2020-05-01\')' } + + it 'sets from to nil' do + expect(subject.from).to be_nil + end + end + + context 'with MAXVALUE as an end date' do + let(:definition) { 'FOR VALUES FROM (\'2020-04-01\') TO (MAXVALUE)' } + + it 'raises a NotImplementedError' do + expect { subject }.to raise_error(NotImplementedError) + end + end + end + + describe '#partition_name' do + subject { described_class.new(table, from, to, partition_name: partition_name).partition_name } + + let(:table) { 'foo' } + let(:from) { '2020-04-01 00:00:00' } + let(:to) { '2020-05-01 00:00:00' } + let(:partition_name) { nil } + + it 'uses table as prefix' do + expect(subject).to start_with(table) + end + + it 'uses Year-Month (from) as suffix' do + expect(subject).to end_with("_202004") + end + + context 'without from date' do + let(:from) { nil } + + it 'uses 000000 as suffix for first partition' do + expect(subject).to end_with("_000000") + end + end + + context 'with partition name explicitly given' do + let(:partition_name) { "foo_bar" } + + it 'uses given partition name' do + expect(subject).to eq(partition_name) + end + end + end + + describe '#to_sql' do + subject { described_class.new(table, from, to).to_sql } + + let(:table) { 'foo' } + let(:from) { '2020-04-01 00:00:00' } + let(:to) { '2020-05-01 00:00:00' } + + it 'transforms to a CREATE TABLE statement' do + expect(subject).to eq(<<~SQL) + CREATE TABLE IF NOT EXISTS "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}"."foo_202004" + PARTITION OF "foo" + FOR VALUES FROM ('2020-04-01') TO ('2020-05-01') + SQL + end + + context 'without from date' do + let(:from) { nil } + + it 'uses MINVALUE instead' do + expect(subject).to eq(<<~SQL) + CREATE TABLE IF NOT EXISTS "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}"."foo_000000" + PARTITION OF "foo" + FOR VALUES FROM (MINVALUE) TO ('2020-05-01') + SQL + end + end + end + + describe 'object equality - #eql' do + def expect_inequality(actual, other) + expect(actual.eql?(other)).to be_falsey + expect(actual).not_to eq(other) + end + + def expect_equality(actual, other) + expect(actual).to eq(other) + expect(actual.eql?(other)).to be_truthy + expect(actual.hash).to eq(other.hash) + end + + def make_new(table: 'foo', from: '2020-04-01 00:00:00', to: '2020-05-01 00:00:00', partition_name: 'foo_202004') + described_class.new(table, from, to, partition_name: partition_name) + end + + it 'treats objects identical with identical attributes' do + expect_equality(make_new, make_new) + end + + it 'different table leads to in-equality' do + expect_inequality(make_new, make_new(table: 'bar')) + end + + it 'different from leads to in-equality' do + expect_inequality(make_new, make_new(from: '2020-05-01 00:00:00')) + end + + it 'different to leads to in-equality' do + expect_inequality(make_new, make_new(to: '2020-06-01 00:00:00')) + end + + it 'different partition_name leads to in-equality' do + expect_inequality(make_new, make_new(partition_name: 'different')) + end + + it 'nil partition_name is ignored if auto-generated matches' do + expect_equality(make_new, make_new(partition_name: nil)) + end + end + + describe 'Comparable, #<=>' do + let(:table) { 'foo' } + + it 'sorts by partition name, i.e. by month - MINVALUE partition first' do + partitions = [ + described_class.new(table, '2020-04-01', '2020-05-01'), + described_class.new(table, '2020-02-01', '2020-03-01'), + described_class.new(table, nil, '2020-02-01'), + described_class.new(table, '2020-03-01', '2020-04-01') + ] + + expect(partitions.sort).to eq([ + described_class.new(table, nil, '2020-02-01'), + described_class.new(table, '2020-02-01', '2020-03-01'), + described_class.new(table, '2020-03-01', '2020-04-01'), + described_class.new(table, '2020-04-01', '2020-05-01') + ]) + end + + it 'returns nil for partitions of different tables' do + one = described_class.new('foo', '2020-02-01', '2020-03-01') + two = described_class.new('bar', '2020-02-01', '2020-03-01') + + expect(one.<=>(two)).to be_nil + end + end +end diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb new file mode 100644 index 00000000000..49f3f87fe61 --- /dev/null +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb @@ -0,0 +1,175 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable, '#perform' do + subject { described_class.new } + + let(:source_table) { '_test_partitioning_backfills' } + let(:destination_table) { "#{source_table}_part" } + let(:unique_key) { 'id' } + + before do + allow(subject).to receive(:transaction_open?).and_return(false) + end + + context 'when the destination table exists' do + before do + connection.execute(<<~SQL) + CREATE TABLE #{source_table} ( + id serial NOT NULL PRIMARY KEY, + col1 int NOT NULL, + col2 text NOT NULL, + created_at timestamptz NOT NULL + ) + SQL + + connection.execute(<<~SQL) + CREATE TABLE #{destination_table} ( + id serial NOT NULL, + col1 int NOT NULL, + col2 text NOT NULL, + created_at timestamptz NOT NULL, + PRIMARY KEY (id, created_at) + ) PARTITION BY RANGE (created_at) + SQL + + connection.execute(<<~SQL) + CREATE TABLE #{destination_table}_202001 PARTITION OF #{destination_table} + FOR VALUES FROM ('2020-01-01') TO ('2020-02-01') + SQL + + connection.execute(<<~SQL) + CREATE TABLE #{destination_table}_202002 PARTITION OF #{destination_table} + FOR VALUES FROM ('2020-02-01') TO ('2020-03-01') + SQL + + source_model.table_name = source_table + destination_model.table_name = destination_table + + stub_const("#{described_class}::SUB_BATCH_SIZE", 2) + stub_const("#{described_class}::PAUSE_SECONDS", pause_seconds) + + allow(subject).to receive(:sleep) + end + + let(:connection) { ActiveRecord::Base.connection } + let(:source_model) { Class.new(ActiveRecord::Base) } + let(:destination_model) { Class.new(ActiveRecord::Base) } + let(:timestamp) { Time.utc(2020, 1, 2).round } + let(:pause_seconds) { 1 } + + let!(:source1) { create_source_record(timestamp) } + let!(:source2) { create_source_record(timestamp + 1.day) } + let!(:source3) { create_source_record(timestamp + 1.month) } + + it 'copies data into the destination table idempotently' do + expect(destination_model.count).to eq(0) + + subject.perform(source1.id, source3.id, source_table, destination_table, unique_key) + + expect(destination_model.count).to eq(3) + + source_model.find_each do |source_record| + destination_record = destination_model.find_by_id(source_record.id) + + expect(destination_record.attributes).to eq(source_record.attributes) + end + + subject.perform(source1.id, source3.id, source_table, destination_table, unique_key) + + expect(destination_model.count).to eq(3) + end + + it 'breaks the assigned batch into smaller batches' do + expect_next_instance_of(described_class::BulkCopy) do |bulk_copy| + expect(bulk_copy).to receive(:copy_between).with(source1.id, source2.id) + expect(bulk_copy).to receive(:copy_between).with(source3.id, source3.id) + end + + subject.perform(source1.id, source3.id, source_table, destination_table, unique_key) + end + + it 'pauses after copying each sub-batch' do + expect(subject).to receive(:sleep).with(pause_seconds).twice + + subject.perform(source1.id, source3.id, source_table, destination_table, unique_key) + end + + it 'marks each job record as succeeded after processing' do + create(:background_migration_job, class_name: "::#{described_class.name}", + arguments: [source1.id, source3.id, source_table, destination_table, unique_key]) + + expect(::Gitlab::Database::BackgroundMigrationJob).to receive(:mark_all_as_succeeded).and_call_original + + expect do + subject.perform(source1.id, source3.id, source_table, destination_table, unique_key) + end.to change { ::Gitlab::Database::BackgroundMigrationJob.succeeded.count }.from(0).to(1) + end + + context 'when the feature flag is disabled' do + let(:mock_connection) { double('connection') } + + before do + allow(subject).to receive(:connection).and_return(mock_connection) + stub_feature_flags(backfill_partitioned_audit_events: false) + end + + it 'exits without attempting to copy data' do + expect(mock_connection).not_to receive(:execute) + + subject.perform(1, 100, source_table, destination_table, unique_key) + + expect(destination_model.count).to eq(0) + end + end + + context 'when the job is run within an explicit transaction block' do + let(:mock_connection) { double('connection') } + + before do + allow(subject).to receive(:connection).and_return(mock_connection) + allow(subject).to receive(:transaction_open?).and_return(true) + end + + it 'raises an error before copying data' do + expect(mock_connection).not_to receive(:execute) + + expect do + subject.perform(1, 100, source_table, destination_table, unique_key) + end.to raise_error(/Aborting job to backfill partitioned #{source_table}/) + + expect(destination_model.count).to eq(0) + end + end + end + + context 'when the destination table does not exist' do + let(:mock_connection) { double('connection') } + let(:mock_logger) { double('logger') } + + before do + allow(subject).to receive(:connection).and_return(mock_connection) + allow(subject).to receive(:logger).and_return(mock_logger) + + expect(mock_connection).to receive(:table_exists?).with(destination_table).and_return(false) + allow(mock_logger).to receive(:warn) + end + + it 'exits without attempting to copy data' do + expect(mock_connection).not_to receive(:execute) + + subject.perform(1, 100, source_table, destination_table, unique_key) + end + + it 'logs a warning message that the job was skipped' do + expect(mock_logger).to receive(:warn).with(/#{destination_table} does not exist/) + + subject.perform(1, 100, source_table, destination_table, unique_key) + end + end + + def create_source_record(timestamp) + source_model.create!(col1: 123, col2: 'original value', created_at: timestamp) + end +end diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb index 9cec77b434d..efa9c83b2d2 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::PartitioningMigrationHelpers::ForeignKeyHelpers do +RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::ForeignKeyHelpers do include TriggerHelpers let(:model) do diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_spec.rb index 77f71676252..a58c37f111d 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::PartitioningMigrationHelpers::PartitionedForeignKey do +RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::PartitionedForeignKey do let(:foreign_key) do described_class.new( to_table: 'issues', diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index 586b57d2002..9b24ab7cad4 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers do +RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers do include PartitioningHelpers include TriggerHelpers @@ -11,7 +11,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end let_it_be(:connection) { ActiveRecord::Base.connection } - let(:template_table) { :audit_events } + let(:source_table) { :audit_events } let(:partitioned_table) { '_test_migration_partitioned_table' } let(:function_name) { '_test_migration_function_name' } let(:trigger_name) { '_test_migration_trigger_name' } @@ -22,10 +22,10 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers before do allow(migration).to receive(:puts) allow(migration).to receive(:transaction_open?).and_return(false) - allow(migration).to receive(:partitioned_table_name).and_return(partitioned_table) - allow(migration).to receive(:sync_function_name).and_return(function_name) - allow(migration).to receive(:sync_trigger_name).and_return(trigger_name) - allow(migration).to receive(:assert_table_is_whitelisted) + allow(migration).to receive(:make_partitioned_table_name).and_return(partitioned_table) + allow(migration).to receive(:make_sync_function_name).and_return(function_name) + allow(migration).to receive(:make_sync_trigger_name).and_return(trigger_name) + allow(migration).to receive(:assert_table_is_allowed) end describe '#partition_table_by_date' do @@ -33,15 +33,19 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers let(:old_primary_key) { 'id' } let(:new_primary_key) { [old_primary_key, partition_column] } - context 'when the table is not whitelisted' do - let(:template_table) { :this_table_is_not_whitelisted } + before do + allow(migration).to receive(:queue_background_migration_jobs_by_range_at_intervals) + end + + context 'when the table is not allowed' do + let(:source_table) { :this_table_is_not_allowed } it 'raises an error' do - expect(migration).to receive(:assert_table_is_whitelisted).with(template_table).and_call_original + expect(migration).to receive(:assert_table_is_allowed).with(source_table).and_call_original expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date - end.to raise_error(/#{template_table} is not whitelisted for use/) + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/#{source_table} is not allowed for use/) end end @@ -50,7 +54,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers expect(migration).to receive(:transaction_open?).and_return(true) expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/can not be run inside a transaction/) end end @@ -60,7 +64,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) end end @@ -70,24 +74,24 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/max_date #{max_date} must be greater than min_date #{min_date}/) end end context 'when the given table does not have a primary key' do - let(:template_table) { :_partitioning_migration_helper_test_table } + let(:source_table) { :_partitioning_migration_helper_test_table } let(:partition_column) { :some_field } it 'raises an error' do - migration.create_table template_table, id: false do |t| + migration.create_table source_table, id: false do |t| t.integer :id t.datetime partition_column end expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date - end.to raise_error(/primary key not defined for #{template_table}/) + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + end.to raise_error(/primary key not defined for #{source_table}/) end end @@ -96,14 +100,14 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers it 'raises an error' do expect do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date end.to raise_error(/partition column #{partition_column} does not exist/) end end describe 'constructing the partitioned table' do it 'creates a table partitioned by the proper column' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect(connection.table_exists?(partitioned_table)).to be(true) expect(connection.primary_key(partitioned_table)).to eq(new_primary_key) @@ -112,7 +116,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end it 'changes the primary key datatype to bigint' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } @@ -127,13 +131,13 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end end - let(:template_table) { :another_example } + let(:source_table) { :another_example } let(:old_primary_key) { 'identifier' } it 'does not change the primary key datatype' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date - original_pk_column = connection.columns(template_table).find { |c| c.name == old_primary_key } + original_pk_column = connection.columns(source_table).find { |c| c.name == old_primary_key } pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } expect(pk_column).not_to be_nil @@ -142,7 +146,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end it 'removes the default from the primary key column' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date pk_column = connection.columns(partitioned_table).find { |c| c.name == old_primary_key } @@ -150,26 +154,100 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end it 'creates the partitioned table with the same non-key columns' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date copied_columns = filter_columns_by_name(connection.columns(partitioned_table), new_primary_key) - original_columns = filter_columns_by_name(connection.columns(template_table), new_primary_key) + original_columns = filter_columns_by_name(connection.columns(source_table), new_primary_key) expect(copied_columns).to match_array(original_columns) end it 'creates a partition spanning over each month in the range given' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + + expect_range_partitions_for(partitioned_table, { + '000000' => ['MINVALUE', "'2019-12-01 00:00:00'"], + '201912' => ["'2019-12-01 00:00:00'", "'2020-01-01 00:00:00'"], + '202001' => ["'2020-01-01 00:00:00'", "'2020-02-01 00:00:00'"], + '202002' => ["'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'"], + '202003' => ["'2020-03-01 00:00:00'", "'2020-04-01 00:00:00'"] + }) + end + + context 'when min_date is not given' do + let(:source_table) { :todos } + + context 'with records present already' do + before do + create(:todo, created_at: Date.parse('2019-11-05')) + end + + it 'creates a partition spanning over each month from the first record' do + migration.partition_table_by_date source_table, partition_column, max_date: max_date + + expect_range_partitions_for(partitioned_table, { + '000000' => ['MINVALUE', "'2019-11-01 00:00:00'"], + '201911' => ["'2019-11-01 00:00:00'", "'2019-12-01 00:00:00'"], + '201912' => ["'2019-12-01 00:00:00'", "'2020-01-01 00:00:00'"], + '202001' => ["'2020-01-01 00:00:00'", "'2020-02-01 00:00:00'"], + '202002' => ["'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'"], + '202003' => ["'2020-03-01 00:00:00'", "'2020-04-01 00:00:00'"] + }) + end + end + + context 'without data' do + it 'creates the catchall partition plus two actual partition' do + migration.partition_table_by_date source_table, partition_column, max_date: max_date + + expect_range_partitions_for(partitioned_table, { + '000000' => ['MINVALUE', "'2020-02-01 00:00:00'"], + '202002' => ["'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'"], + '202003' => ["'2020-03-01 00:00:00'", "'2020-04-01 00:00:00'"] + }) + end + end + end - expect_range_partition_of("#{partitioned_table}_000000", partitioned_table, 'MINVALUE', "'2019-12-01 00:00:00'") - expect_range_partition_of("#{partitioned_table}_201912", partitioned_table, "'2019-12-01 00:00:00'", "'2020-01-01 00:00:00'") - expect_range_partition_of("#{partitioned_table}_202001", partitioned_table, "'2020-01-01 00:00:00'", "'2020-02-01 00:00:00'") - expect_range_partition_of("#{partitioned_table}_202002", partitioned_table, "'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'") + context 'when max_date is not given' do + it 'creates partitions including the next month from today' do + today = Date.new(2020, 5, 8) + + Timecop.freeze(today) do + migration.partition_table_by_date source_table, partition_column, min_date: min_date + + expect_range_partitions_for(partitioned_table, { + '000000' => ['MINVALUE', "'2019-12-01 00:00:00'"], + '201912' => ["'2019-12-01 00:00:00'", "'2020-01-01 00:00:00'"], + '202001' => ["'2020-01-01 00:00:00'", "'2020-02-01 00:00:00'"], + '202002' => ["'2020-02-01 00:00:00'", "'2020-03-01 00:00:00'"], + '202003' => ["'2020-03-01 00:00:00'", "'2020-04-01 00:00:00'"], + '202004' => ["'2020-04-01 00:00:00'", "'2020-05-01 00:00:00'"], + '202005' => ["'2020-05-01 00:00:00'", "'2020-06-01 00:00:00'"], + '202006' => ["'2020-06-01 00:00:00'", "'2020-07-01 00:00:00'"] + }) + end + end + end + + context 'without min_date, max_date' do + it 'creates partitions for the current and next month' do + current_date = Date.new(2020, 05, 22) + Timecop.freeze(current_date.to_time) do + migration.partition_table_by_date source_table, partition_column + + expect_range_partitions_for(partitioned_table, { + '000000' => ['MINVALUE', "'2020-05-01 00:00:00'"], + '202005' => ["'2020-05-01 00:00:00'", "'2020-06-01 00:00:00'"], + '202006' => ["'2020-06-01 00:00:00'", "'2020-07-01 00:00:00'"] + }) + end + end end end describe 'keeping data in sync with the partitioned table' do - let(:template_table) { :todos } + let(:source_table) { :todos } let(:model) { Class.new(ActiveRecord::Base) } let(:timestamp) { Time.utc(2019, 12, 1, 12).round } @@ -180,16 +258,16 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers it 'creates a trigger function on the original table' do expect_function_not_to_exist(function_name) - expect_trigger_not_to_exist(template_table, trigger_name) + expect_trigger_not_to_exist(source_table, trigger_name) - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect_function_to_exist(function_name) - expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) end it 'syncs inserts to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect(model.count).to eq(0) @@ -202,7 +280,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end it 'syncs updates to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date first_todo = create(:todo, :pending, commit_id: nil, created_at: timestamp, updated_at: timestamp) second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) @@ -223,7 +301,7 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers end it 'syncs deletes to the partitioned tables' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date first_todo = create(:todo, created_at: timestamp, updated_at: timestamp) second_todo = create(:todo, created_at: timestamp, updated_at: timestamp) @@ -237,50 +315,129 @@ describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers expect(model.find(second_todo.id).attributes).to eq(second_todo.attributes) end end + + describe 'copying historic data to the partitioned table' do + let(:source_table) { 'todos' } + let(:migration_class) { '::Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable' } + let(:sub_batch_size) { described_class::SUB_BATCH_SIZE } + let(:pause_seconds) { described_class::PAUSE_SECONDS } + let!(:first_id) { create(:todo).id } + let!(:second_id) { create(:todo).id } + let!(:third_id) { create(:todo).id } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + expect(migration).to receive(:queue_background_migration_jobs_by_range_at_intervals).and_call_original + end + + it 'enqueues jobs to copy each batch of data' do + Sidekiq::Testing.fake! do + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + + first_job_arguments = [first_id, second_id, source_table, partitioned_table, 'id'] + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq([migration_class, first_job_arguments]) + + second_job_arguments = [third_id, third_id, source_table, partitioned_table, 'id'] + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq([migration_class, second_job_arguments]) + end + end + end end describe '#drop_partitioned_table_for' do let(:expected_tables) do - %w[000000 201912 202001 202002].map { |suffix| "#{partitioned_table}_#{suffix}" }.unshift(partitioned_table) + %w[000000 201912 202001 202002].map { |suffix| "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.#{partitioned_table}_#{suffix}" }.unshift(partitioned_table) end + let(:migration_class) { 'Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable' } - context 'when the table is not whitelisted' do - let(:template_table) { :this_table_is_not_whitelisted } + context 'when the table is not allowed' do + let(:source_table) { :this_table_is_not_allowed } it 'raises an error' do - expect(migration).to receive(:assert_table_is_whitelisted).with(template_table).and_call_original + expect(migration).to receive(:assert_table_is_allowed).with(source_table).and_call_original expect do - migration.drop_partitioned_table_for template_table - end.to raise_error(/#{template_table} is not whitelisted for use/) + migration.drop_partitioned_table_for source_table + end.to raise_error(/#{source_table} is not allowed for use/) end end it 'drops the trigger syncing to the partitioned table' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expect_function_to_exist(function_name) - expect_valid_function_trigger(template_table, trigger_name, function_name, after: %w[delete insert update]) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) - migration.drop_partitioned_table_for template_table + migration.drop_partitioned_table_for source_table expect_function_not_to_exist(function_name) - expect_trigger_not_to_exist(template_table, trigger_name) + expect_trigger_not_to_exist(source_table, trigger_name) end it 'drops the partitioned copy and all partitions' do - migration.partition_table_by_date template_table, partition_column, min_date: min_date, max_date: max_date + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date expected_tables.each do |table| expect(connection.table_exists?(table)).to be(true) end - migration.drop_partitioned_table_for template_table + migration.drop_partitioned_table_for source_table expected_tables.each do |table| expect(connection.table_exists?(table)).to be(false) end end + + context 'cleaning up background migration tracking records' do + let!(:job1) { create(:background_migration_job, class_name: migration_class, arguments: [1, 10, source_table]) } + let!(:job2) { create(:background_migration_job, class_name: migration_class, arguments: [11, 20, source_table]) } + let!(:job3) { create(:background_migration_job, class_name: migration_class, arguments: [1, 10, 'other_table']) } + + it 'deletes any tracking records from the background_migration_jobs table' do + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + + expect { migration.drop_partitioned_table_for(source_table) } + .to change { ::Gitlab::Database::BackgroundMigrationJob.count }.from(3).to(1) + + remaining_record = ::Gitlab::Database::BackgroundMigrationJob.first + expect(remaining_record).to have_attributes(class_name: migration_class, arguments: [1, 10, 'other_table']) + end + end + end + + describe '#create_hash_partitions' do + before do + connection.execute(<<~SQL) + CREATE TABLE #{partitioned_table} + (id serial not null, some_id integer not null, PRIMARY KEY (id, some_id)) + PARTITION BY HASH (some_id); + SQL + end + + it 'creates partitions for the full hash space (8 partitions)' do + partitions = 8 + + migration.create_hash_partitions(partitioned_table, partitions) + + (0..partitions - 1).each do |partition| + partition_name = "#{partitioned_table}_#{"%01d" % partition}" + expect_hash_partition_of(partition_name, partitioned_table, partitions, partition) + end + end + + it 'creates partitions for the full hash space (16 partitions)' do + partitions = 16 + + migration.create_hash_partitions(partitioned_table, partitions) + + (0..partitions - 1).each do |partition| + partition_name = "#{partitioned_table}_#{"%02d" % partition}" + expect_hash_partition_of(partition_name, partitioned_table, partitions, partition) + end + end end def filter_columns_by_name(columns, names) diff --git a/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb b/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb index 0523066b593..8b3a0ceb804 100644 --- a/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb +++ b/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::PostgresqlAdapter::ForceDisconnectableMixin do +RSpec.describe Gitlab::Database::PostgresqlAdapter::ForceDisconnectableMixin do describe 'checking in a connection to the pool' do let(:model) do Class.new(ActiveRecord::Base) do diff --git a/spec/lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin_spec.rb b/spec/lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin_spec.rb index 968dfc1ea43..c6333e4a4dc 100644 --- a/spec/lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin_spec.rb +++ b/spec/lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::PostgresqlAdapter::SchemaVersionsCopyMixin do +RSpec.describe Gitlab::Database::PostgresqlAdapter::SchemaVersionsCopyMixin do let(:schema_migration) { double('schem_migration', table_name: table_name, all_versions: versions) } let(:versions) { %w(5 2 1000 200 4 93 2) } let(:table_name) { "schema_migrations" } diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb index fae57996fb6..76b1be1e497 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :delete do +RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :delete do let(:migration) { FakeRenameReservedPathMigrationV1.new } let(:subject) { described_class.new(['the-path'], migration) } diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb index 46fc48ab3fc..e222a29c6a1 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :delete do +RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :delete do let(:migration) { FakeRenameReservedPathMigrationV1.new } let(:subject) { described_class.new(['the-path'], migration) } let(:namespace) { create(:group, name: 'the-path') } diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb index 1ccdb1d9447..3799fe3c316 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :delete do +RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :delete do let(:migration) { FakeRenameReservedPathMigrationV1.new } let(:subject) { described_class.new(['the-path'], migration) } let(:project) do diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb index 56767c21ab7..3b2d3ab1354 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -shared_examples 'renames child namespaces' do |type| +RSpec.shared_examples 'renames child namespaces' do |type| it 'renames namespaces' do rename_namespaces = double expect(described_class::RenameNamespaces) @@ -15,7 +15,7 @@ shared_examples 'renames child namespaces' do |type| end end -describe Gitlab::Database::RenameReservedPathsMigration::V1, :delete do +RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1, :delete do let(:subject) { FakeRenameReservedPathMigrationV1.new } before do diff --git a/spec/lib/gitlab/database/schema_cleaner_spec.rb b/spec/lib/gitlab/database/schema_cleaner_spec.rb index adaeb85d52d..1303ad7a311 100644 --- a/spec/lib/gitlab/database/schema_cleaner_spec.rb +++ b/spec/lib/gitlab/database/schema_cleaner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::SchemaCleaner do +RSpec.describe Gitlab::Database::SchemaCleaner do let(:example_schema) { fixture_file(File.join('gitlab', 'database', 'structure_example.sql')) } let(:io) { StringIO.new } diff --git a/spec/lib/gitlab/database/sha_attribute_spec.rb b/spec/lib/gitlab/database/sha_attribute_spec.rb index 15695bc8069..1b855625a6c 100644 --- a/spec/lib/gitlab/database/sha_attribute_spec.rb +++ b/spec/lib/gitlab/database/sha_attribute_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::ShaAttribute do +RSpec.describe Gitlab::Database::ShaAttribute do let(:sha) do '9a573a369a5bfbb9a4a36e98852c21af8a44ea8b' end diff --git a/spec/lib/gitlab/database/with_lock_retries_spec.rb b/spec/lib/gitlab/database/with_lock_retries_spec.rb index d7eee594631..70cbddbb7b7 100644 --- a/spec/lib/gitlab/database/with_lock_retries_spec.rb +++ b/spec/lib/gitlab/database/with_lock_retries_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Database::WithLockRetries do +RSpec.describe Gitlab::Database::WithLockRetries do let(:env) { {} } let(:logger) { Gitlab::Database::WithLockRetries::NULL_LOGGER } let(:subject) { described_class.new(env: env, logger: logger, timing_configuration: timing_configuration) } |