diff --git a/ee/app/workers/elastic/migration_worker.rb b/ee/app/workers/elastic/migration_worker.rb index 914be2a4de7f7e5ccb2ce9c158bcbfe0945e708e..785c5bc5972a576ddcdb0ac1ec71dfc1f1338ab4 100644 --- a/ee/app/workers/elastic/migration_worker.rb +++ b/ee/app/workers/elastic/migration_worker.rb @@ -87,9 +87,9 @@ def execute_migration(migration) end def current_migration - completed_migrations = Elastic::MigrationRecord.load_versions(completed: true) + uncompleted_migrations = Elastic::MigrationRecord.load_versions(completed: false) - Elastic::DataMigrationService.migrations.find { |migration| !completed_migrations.include?(migration.version) } + Elastic::DataMigrationService.migrations.find { |migration| uncompleted_migrations.include?(migration.version) } end def pause_indexing!(migration) diff --git a/ee/spec/workers/elastic/migration_worker_spec.rb b/ee/spec/workers/elastic/migration_worker_spec.rb index 33b87d7cf29598a909c18b4d24f39b500563450c..ab6d2b6d65981d7d470306ca4024f4b4661165b8 100644 --- a/ee/spec/workers/elastic/migration_worker_spec.rb +++ b/ee/spec/workers/elastic/migration_worker_spec.rb @@ -22,168 +22,184 @@ before do stub_ee_application_setting(elasticsearch_indexing: true) - - allow(subject).to receive(:current_migration).and_return(migration) end - it 'creates an index if it does not exist' do - Gitlab::Elastic::Helper.default.delete_migrations_index - - expect { subject.perform }.to change { Gitlab::Elastic::Helper.default.migrations_index_exists? }.from(false).to(true) - end - - context 'no unexecuted migrations' do + context 'an unexecuted migration present' do before do - allow(subject).to receive(:current_migration).and_return(nil) + allow(subject).to receive(:current_migration).and_return(migration) end - it 'skips execution' do - expect(subject).not_to receive(:execute_migration) + it 'creates an index if it does not exist' do + Gitlab::Elastic::Helper.default.delete_migrations_index - expect(subject.perform).to be_falsey + expect { subject.perform }.to change { Gitlab::Elastic::Helper.default.migrations_index_exists? }.from(false).to(true) end - end - context 'migration is halted' do - using RSpec::Parameterized::TableSyntax + context 'migration is halted' do + using RSpec::Parameterized::TableSyntax - where(:pause_indexing, :halted_indexing_unpaused, :unpause) do - false | false | false - false | true | false - true | false | true - true | true | false - end - - with_them do - before do - allow(Gitlab::CurrentSettings).to receive(:elasticsearch_pause_indexing?).and_return(true) - allow(migration).to receive(:pause_indexing?).and_return(true) - migration.save_state!(halted: true, pause_indexing: pause_indexing, halted_indexing_unpaused: halted_indexing_unpaused) + where(:pause_indexing, :halted_indexing_unpaused, :unpause) do + false | false | false + false | true | false + true | false | true + true | true | false end - it 'unpauses indexing' do - if unpause - expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false) - else - expect(Gitlab::CurrentSettings).not_to receive(:update!) + with_them do + before do + allow(Gitlab::CurrentSettings).to receive(:elasticsearch_pause_indexing?).and_return(true) + allow(migration).to receive(:pause_indexing?).and_return(true) + migration.save_state!(halted: true, pause_indexing: pause_indexing, halted_indexing_unpaused: halted_indexing_unpaused) end - expect(migration).not_to receive(:migrate) + it 'unpauses indexing' do + if unpause + expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false) + else + expect(Gitlab::CurrentSettings).not_to receive(:update!) + end - subject.perform + expect(migration).not_to receive(:migrate) + + subject.perform + end end end - end - context 'migration process' do - before do - allow(migration).to receive(:started?).and_return(started) - allow(migration).to receive(:completed?).and_return(completed) - allow(migration).to receive(:batched?).and_return(batched) - end + context 'migration process' do + before do + allow(migration).to receive(:started?).and_return(started) + allow(migration).to receive(:completed?).and_return(completed) + allow(migration).to receive(:batched?).and_return(batched) + end - using RSpec::Parameterized::TableSyntax - - # completed is evaluated after migrate method is executed - where(:started, :completed, :execute_migration, :batched) do - false | false | true | false - false | true | true | false - false | false | true | true - false | true | true | true - true | false | false | false - true | true | false | false - true | false | true | true - true | true | true | true - end + using RSpec::Parameterized::TableSyntax + + # completed is evaluated after migrate method is executed + where(:started, :completed, :execute_migration, :batched) do + false | false | true | false + false | true | true | false + false | false | true | true + false | true | true | true + true | false | false | false + true | true | false | false + true | false | true | true + true | true | true | true + end - with_them do - it 'calls migration only when needed', :aggregate_failures do - if execute_migration - expect(migration).to receive(:migrate).once - else - expect(migration).not_to receive(:migrate) + with_them do + it 'calls migration only when needed', :aggregate_failures do + if execute_migration + expect(migration).to receive(:migrate).once + else + expect(migration).not_to receive(:migrate) + end + + expect(migration).to receive(:save!).with(completed: completed) + expect(Elastic::DataMigrationService).to receive(:drop_migration_has_finished_cache!).with(migration) + + subject.perform end - expect(migration).to receive(:save!).with(completed: completed) - expect(Elastic::DataMigrationService).to receive(:drop_migration_has_finished_cache!).with(migration) + it 'handles batched migrations' do + if batched && !completed + # default throttle_delay is 5.minutes + expect( Elastic::MigrationWorker).to receive(:perform_in) + .with(5.minutes) + else + expect( Elastic::MigrationWorker).not_to receive(:perform_in) + end - subject.perform + subject.perform + end end - it 'handles batched migrations' do - if batched && !completed - # default throttle_delay is 5.minutes - expect( Elastic::MigrationWorker).to receive(:perform_in) - .with(5.minutes) - else - expect( Elastic::MigrationWorker).not_to receive(:perform_in) + context 'indexing pause' do + before do + allow(migration).to receive(:pause_indexing?).and_return(true) end - subject.perform - end - end + let(:batched) { true } - context 'indexing pause' do - before do - allow(migration).to receive(:pause_indexing?).and_return(true) - end + where(:started, :completed, :expected) do + false | false | false + true | false | false + true | true | true + end - let(:batched) { true } + with_them do + it 'pauses and unpauses indexing' do + expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: true) + expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false) if expected - where(:started, :completed, :expected) do - false | false | false - true | false | false - true | true | true + subject.perform + end + end end - with_them do - it 'pauses and unpauses indexing' do - expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: true) - expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false) if expected + context 'checks space required' do + let(:helper) { Gitlab::Elastic::Helper.new } + let(:started) { false } + let(:completed) { false } + let(:batched) { false } + + before do + allow(Gitlab::Elastic::Helper).to receive(:default).and_return(helper) + allow(migration).to receive(:space_requirements?).and_return(true) + allow(migration).to receive(:space_required_bytes).and_return(10) + end + + it 'halts the migration if there is not enough space' do + allow(helper).to receive(:cluster_free_size_bytes).and_return(5) + expect(migration).to receive(:halt!) + expect(migration).not_to receive(:migrate) subject.perform end - end - end - context 'checks space required' do - let(:helper) { Gitlab::Elastic::Helper.new } - let(:started) { false } - let(:completed) { false } - let(:batched) { false } + it 'runs the migration if there is enough space' do + allow(helper).to receive(:cluster_free_size_bytes).and_return(20) + expect(migration).not_to receive(:halt!) + expect(migration).to receive(:migrate).once - before do - allow(Gitlab::Elastic::Helper).to receive(:default).and_return(helper) - allow(migration).to receive(:space_requirements?).and_return(true) - allow(migration).to receive(:space_required_bytes).and_return(10) - end + subject.perform + end - it 'halts the migration if there is not enough space' do - allow(helper).to receive(:cluster_free_size_bytes).and_return(5) - expect(migration).to receive(:halt!) - expect(migration).not_to receive(:migrate) + context 'when migration is already started' do + let(:started) { true } - subject.perform + it 'does not check space requirements' do + expect(helper).not_to receive(:cluster_free_size_bytes) + expect(migration).not_to receive(:space_required_bytes) + + subject.perform + end + end end + end + end - it 'runs the migration if there is enough space' do - allow(helper).to receive(:cluster_free_size_bytes).and_return(20) - expect(migration).not_to receive(:halt!) - expect(migration).to receive(:migrate).once + context 'no unexecuted migrations' do + before do + allow(subject).to receive(:current_migration).and_return(nil) + end - subject.perform - end + it 'skips execution' do + expect(subject).not_to receive(:execute_migration) - context 'when migration is already started' do - let(:started) { true } + expect(subject.perform).to be_falsey + end + end - it 'does not check space requirements' do - expect(helper).not_to receive(:cluster_free_size_bytes) - expect(migration).not_to receive(:space_required_bytes) + context 'load_versions returns empty array' do + before do + allow(Elastic::MigrationRecord).to receive(:load_versions).and_return([]) + end - subject.perform - end - end + it 'skips execution' do + expect(subject).not_to receive(:execute_migration) + + expect(subject.perform).to be_falsey end end end