Commit 0f9f1b99 authored by Terri Chu's avatar Terri Chu Committed by Dmitry Gruzd

Fix Elastic::MigrationWorker current_migration (2nd attempt)

parent dec0df95
......@@ -65,8 +65,6 @@ module Elastic
.search(index: helper.migrations_index_name, body: { query: { term: { completed: completed } }, size: ELASTICSEARCH_SIZE })
.dig('hits', 'hits')
.map { |v| v['_id'].to_i }
rescue StandardError
[]
end
def running?
......
......@@ -21,6 +21,13 @@ module Elastic
return false unless helper.alias_exists?
in_lock(self.class.name.underscore, ttl: 1.day, retries: 10, sleep_sec: 1) do
# migration index should be checked before pulling the current_migration because if no migrations_index exists,
# current_migration will return nil
unless helper.migrations_index_exists?
logger.info 'MigrationWorker: creating migrations index'
helper.create_migrations_index
end
migration = current_migration
unless migration
......@@ -28,11 +35,6 @@ module Elastic
break false
end
unless helper.migrations_index_exists?
logger.info 'MigrationWorker: creating migrations index'
helper.create_migrations_index
end
if migration.halted?
logger.info "MigrationWorker: migration[#{migration.name}] has been halted. All future migrations will be halted because of that. Exiting"
unpause_indexing!(migration)
......@@ -89,7 +91,12 @@ module Elastic
def current_migration
completed_migrations = Elastic::MigrationRecord.load_versions(completed: true)
# use a negative condition to support new migrations which do not exist in the index yet
Elastic::DataMigrationService.migrations.find { |migration| !completed_migrations.include?(migration.version) }
rescue StandardError => e
# do not return a migration if there is an issue communicating with the Elasticsearch instance
logger.error("MigrationWorker: #{e.class}: #{e.message}")
nil
end
def pause_indexing!(migration)
......
......@@ -42,13 +42,13 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
end
describe '#load_from_index' do
it 'does not raise an exeption when connection refused' do
it 'does not raise an exception when connection refused' do
allow(Gitlab::Elastic::Helper.default).to receive(:get).and_raise(Faraday::ConnectionFailed)
expect(record.load_from_index).to be_nil
end
it 'does not raise an exeption when record does not exist' do
it 'does not raise an exception when record does not exist' do
allow(Gitlab::Elastic::Helper.default).to receive(:get).and_raise(Elasticsearch::Transport::Transport::Errors::NotFound)
expect(record.load_from_index).to be_nil
......@@ -95,18 +95,18 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
expect(described_class.load_versions(completed: false)).to contain_exactly(in_progress_migration.version)
end
it 'returns empty array if no index present' do
it 'raises an exception if no index present' do
es_helper.delete_migrations_index
expect(described_class.load_versions(completed: true)).to eq([])
expect(described_class.load_versions(completed: false)).to eq([])
expect { described_class.load_versions(completed: true) }.to raise_exception(Elasticsearch::Transport::Transport::Errors::NotFound)
expect { described_class.load_versions(completed: false) }.to raise_exception(Elasticsearch::Transport::Transport::Errors::NotFound)
end
it 'returns empty array when exception is raised' do
it 'raises an exception when exception is raised' do
allow(Gitlab::Elastic::Helper.default.client).to receive(:search).and_raise(Faraday::ConnectionFailed)
expect(described_class.load_versions(completed: true)).to eq([])
expect(described_class.load_versions(completed: false)).to eq([])
expect { described_class.load_versions(completed: true) }.to raise_exception(StandardError)
expect { described_class.load_versions(completed: false) }.to raise_exception(StandardError)
end
end
......
......@@ -69,7 +69,6 @@ RSpec.describe 'gitlab:elastic namespace rake tasks', :elastic, :silence_stdout
it 'marks all migrations as completed' do
expect(Elastic::DataMigrationService).to receive(:mark_all_as_completed!).and_call_original
expect(Elastic::MigrationRecord.load_versions(completed: true)).to eq([])
subject
refresh_index!
......
......@@ -22,7 +22,10 @@ RSpec.describe Elastic::MigrationWorker, :elastic do
before do
stub_ee_application_setting(elasticsearch_indexing: true)
end
context 'an unexecuted migration present' do
before do
allow(subject).to receive(:current_migration).and_return(migration)
end
......@@ -32,18 +35,6 @@ RSpec.describe Elastic::MigrationWorker, :elastic do
expect { subject.perform }.to change { Gitlab::Elastic::Helper.default.migrations_index_exists? }.from(false).to(true)
end
context 'no unexecuted migrations' do
before do
allow(subject).to receive(:current_migration).and_return(nil)
end
it 'skips execution' do
expect(subject).not_to receive(:execute_migration)
expect(subject.perform).to be_falsey
end
end
context 'migration is halted' do
using RSpec::Parameterized::TableSyntax
......@@ -187,5 +178,31 @@ RSpec.describe Elastic::MigrationWorker, :elastic do
end
end
end
context 'no unexecuted migrations' do
before do
allow(subject).to receive(:current_migration).and_return(nil)
end
it 'skips execution' do
expect(subject).not_to receive(:execute_migration)
expect(subject.perform).to be_falsey
end
end
context 'no executed migrations' do
before do
allow(Elastic::MigrationRecord).to receive(:load_versions).and_return([])
allow(Elastic::DataMigrationService).to receive(:migrations).and_return([migration])
end
it 'executes the first migration' do
expect(subject).to receive(:execute_migration).with(migration)
subject.perform
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment