Commit e7a52f0d authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch...

Merge branch '355975-pause-the-execution-of-a-batched-background-migration-when-too-many-jobs-fail' into 'master'

Resolve "Fail the execution of a batched background migration when too many jobs fail"

See merge request gitlab-org/gitlab!82993
parents 1ef39a79 e61ba7c9
...@@ -6,8 +6,8 @@ class Admin::BackgroundMigrationsController < Admin::ApplicationController ...@@ -6,8 +6,8 @@ class Admin::BackgroundMigrationsController < Admin::ApplicationController
def index def index
@relations_by_tab = { @relations_by_tab = {
'queued' => batched_migration_class.queued.queue_order, 'queued' => batched_migration_class.queued.queue_order,
'failed' => batched_migration_class.failed.queue_order, 'failed' => batched_migration_class.with_status(:failed).queue_order,
'finished' => batched_migration_class.finished.queue_order.reverse_order 'finished' => batched_migration_class.with_status(:finished).queue_order.reverse_order
} }
@current_tab = @relations_by_tab.key?(params[:tab]) ? params[:tab] : 'queued' @current_tab = @relations_by_tab.key?(params[:tab]) ? params[:tab] : 'queued'
...@@ -17,14 +17,14 @@ class Admin::BackgroundMigrationsController < Admin::ApplicationController ...@@ -17,14 +17,14 @@ class Admin::BackgroundMigrationsController < Admin::ApplicationController
def pause def pause
migration = batched_migration_class.find(params[:id]) migration = batched_migration_class.find(params[:id])
migration.paused! migration.pause!
redirect_back fallback_location: { action: 'index' } redirect_back fallback_location: { action: 'index' }
end end
def resume def resume
migration = batched_migration_class.find(params[:id]) migration = batched_migration_class.find(params[:id])
migration.active! migration.execute!
redirect_back fallback_location: { action: 'index' } redirect_back fallback_location: { action: 'index' }
end end
......
...@@ -4,13 +4,13 @@ module Admin ...@@ -4,13 +4,13 @@ module Admin
module BackgroundMigrationsHelper module BackgroundMigrationsHelper
def batched_migration_status_badge_variant(migration) def batched_migration_status_badge_variant(migration)
variants = { variants = {
'active' => :info, active: :info,
'paused' => :warning, paused: :warning,
'failed' => :danger, failed: :danger,
'finished' => :success finished: :success
} }
variants[migration.status] variants[migration.status_name]
end end
# The extra logic here is needed because total_tuple_count is just # The extra logic here is needed because total_tuple_count is just
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
- else - else
= _('Unknown') = _('Unknown')
%td{ role: 'cell', data: { label: _('Status') } } %td{ role: 'cell', data: { label: _('Status') } }
= gl_badge_tag migration.status.humanize, { size: :sm, variant: batched_migration_status_badge_variant(migration) } = gl_badge_tag migration.status_name.to_s.humanize, { size: :sm, variant: batched_migration_status_badge_variant(migration) }
%td{ role: 'cell', data: { label: _('Action') } } %td{ role: 'cell', data: { label: _('Action') } }
- if migration.active? - if migration.active?
= button_to pause_admin_background_migration_path(migration), = button_to pause_admin_background_migration_path(migration),
......
# frozen_string_literal: true
class AddStartedAtToBatchedBackgroundMigrationsTable < Gitlab::Database::Migration[1.0]
def change
add_column :batched_background_migrations, :started_at, :datetime_with_timezone
end
end
0e8c43f8878152e355a435dc0c72874b9808aba376e6969bec85a33ec5c01b1c
\ No newline at end of file
...@@ -11715,6 +11715,7 @@ CREATE TABLE batched_background_migrations ( ...@@ -11715,6 +11715,7 @@ CREATE TABLE batched_background_migrations (
total_tuple_count bigint, total_tuple_count bigint,
pause_ms integer DEFAULT 100 NOT NULL, pause_ms integer DEFAULT 100 NOT NULL,
max_batch_size integer, max_batch_size integer,
started_at timestamp with time zone,
CONSTRAINT check_5bb0382d6f CHECK ((char_length(column_name) <= 63)), CONSTRAINT check_5bb0382d6f CHECK ((char_length(column_name) <= 63)),
CONSTRAINT check_6b6a06254a CHECK ((char_length(table_name) <= 63)), CONSTRAINT check_6b6a06254a CHECK ((char_length(table_name) <= 63)),
CONSTRAINT check_batch_size_in_range CHECK ((batch_size >= sub_batch_size)), CONSTRAINT check_batch_size_in_range CHECK ((batch_size >= sub_batch_size)),
...@@ -25,6 +25,7 @@ module Gitlab ...@@ -25,6 +25,7 @@ module Gitlab
scope :except_succeeded, -> { without_status(:succeeded) } scope :except_succeeded, -> { without_status(:succeeded) }
scope :successful_in_execution_order, -> { where.not(finished_at: nil).with_status(:succeeded).order(:finished_at) } scope :successful_in_execution_order, -> { where.not(finished_at: nil).with_status(:succeeded).order(:finished_at) }
scope :with_preloads, -> { preload(:batched_migration) } scope :with_preloads, -> { preload(:batched_migration) }
scope :created_since, ->(date_time) { where('created_at >= ?', date_time) }
state_machine :status, initial: :pending do state_machine :status, initial: :pending do
state :pending, value: 0 state :pending, value: 0
......
...@@ -6,6 +6,8 @@ module Gitlab ...@@ -6,6 +6,8 @@ module Gitlab
class BatchedMigration < SharedModel class BatchedMigration < SharedModel
JOB_CLASS_MODULE = 'Gitlab::BackgroundMigration' JOB_CLASS_MODULE = 'Gitlab::BackgroundMigration'
BATCH_CLASS_MODULE = "#{JOB_CLASS_MODULE}::BatchingStrategies" BATCH_CLASS_MODULE = "#{JOB_CLASS_MODULE}::BatchingStrategies"
MAXIMUM_FAILED_RATIO = 0.5
MINIMUM_JOBS = 50
self.table_name = :batched_background_migrations self.table_name = :batched_background_migrations
...@@ -21,28 +23,56 @@ module Gitlab ...@@ -21,28 +23,56 @@ module Gitlab
validate :validate_batched_jobs_status, if: -> { status_changed? && finished? } validate :validate_batched_jobs_status, if: -> { status_changed? && finished? }
scope :queue_order, -> { order(id: :asc) } scope :queue_order, -> { order(id: :asc) }
scope :queued, -> { where(status: [:active, :paused]) } scope :queued, -> { with_statuses(:active, :paused) }
scope :for_configuration, ->(job_class_name, table_name, column_name, job_arguments) do scope :for_configuration, ->(job_class_name, table_name, column_name, job_arguments) do
where(job_class_name: job_class_name, table_name: table_name, column_name: column_name) where(job_class_name: job_class_name, table_name: table_name, column_name: column_name)
.where("job_arguments = ?", job_arguments.to_json) # rubocop:disable Rails/WhereEquals .where("job_arguments = ?", job_arguments.to_json) # rubocop:disable Rails/WhereEquals
end end
enum status: { state_machine :status, initial: :paused do
paused: 0, state :paused, value: 0
active: 1, state :active, value: 1
finished: 3, state :finished, value: 3
failed: 4, state :failed, value: 4
finalizing: 5 state :finalizing, value: 5
}
event :pause do
transition any => :paused
end
event :execute do
transition any => :active
end
event :finish do
transition any => :finished
end
event :failure do
transition any => :failed
end
event :finalize do
transition any => :finalizing
end
before_transition any => :active do |migration|
migration.started_at = Time.current if migration.respond_to?(:started_at)
end
end
attribute :pause_ms, :integer, default: 100 attribute :pause_ms, :integer, default: 100
def self.valid_status
state_machine.states.map(&:name)
end
def self.find_for_configuration(job_class_name, table_name, column_name, job_arguments) def self.find_for_configuration(job_class_name, table_name, column_name, job_arguments)
for_configuration(job_class_name, table_name, column_name, job_arguments).first for_configuration(job_class_name, table_name, column_name, job_arguments).first
end end
def self.active_migration def self.active_migration
active.queue_order.first with_status(:active).queue_order.first
end end
def self.successful_rows_counts(migrations) def self.successful_rows_counts(migrations)
...@@ -74,11 +104,23 @@ module Gitlab ...@@ -74,11 +104,23 @@ module Gitlab
batched_jobs.with_status(:failed).each_batch(of: 100) do |batch| batched_jobs.with_status(:failed).each_batch(of: 100) do |batch|
self.class.transaction do self.class.transaction do
batch.lock.each(&:split_and_retry!) batch.lock.each(&:split_and_retry!)
self.active! self.execute!
end end
end end
self.active! self.execute!
end
def should_stop?
return unless started_at
total_jobs = batched_jobs.created_since(started_at).count
return if total_jobs < MINIMUM_JOBS
failed_jobs = batched_jobs.with_status(:failed).created_since(started_at).count
failed_jobs.fdiv(total_jobs) > MAXIMUM_FAILED_RATIO
end end
def next_min_value def next_min_value
......
...@@ -30,6 +30,7 @@ module Gitlab ...@@ -30,6 +30,7 @@ module Gitlab
migration_wrapper.perform(next_batched_job) migration_wrapper.perform(next_batched_job)
active_migration.optimize! active_migration.optimize!
active_migration.failure! if next_batched_job.failed? && active_migration.should_stop?
else else
finish_active_migration(active_migration) finish_active_migration(active_migration)
end end
...@@ -67,7 +68,7 @@ module Gitlab ...@@ -67,7 +68,7 @@ module Gitlab
elsif migration.finished? elsif migration.finished?
Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}" Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}"
else else
migration.finalizing! migration.finalize!
migration.batched_jobs.with_status(:pending).each { |job| migration_wrapper.perform(job) } migration.batched_jobs.with_status(:pending).each { |job| migration_wrapper.perform(job) }
run_migration_while(migration, :finalizing) run_migration_while(migration, :finalizing)
...@@ -118,14 +119,14 @@ module Gitlab ...@@ -118,14 +119,14 @@ module Gitlab
return if active_migration.batched_jobs.active.exists? return if active_migration.batched_jobs.active.exists?
if active_migration.batched_jobs.with_status(:failed).exists? if active_migration.batched_jobs.with_status(:failed).exists?
active_migration.failed! active_migration.failure!
else else
active_migration.finished! active_migration.finish!
end end
end end
def run_migration_while(migration, status) def run_migration_while(migration, status)
while migration.status == status.to_s while migration.status_name == status
run_migration_job(migration) run_migration_job(migration)
migration.reload_last_job migration.reload_last_job
......
...@@ -956,7 +956,7 @@ module Gitlab ...@@ -956,7 +956,7 @@ module Gitlab
Gitlab::AppLogger.warn "Could not find batched background migration for the given configuration: #{configuration}" Gitlab::AppLogger.warn "Could not find batched background migration for the given configuration: #{configuration}"
elsif !migration.finished? elsif !migration.finished?
raise "Expected batched background migration for the given configuration to be marked as 'finished', " \ raise "Expected batched background migration for the given configuration to be marked as 'finished', " \
"but it is '#{migration.status}':" \ "but it is '#{migration.status_name}':" \
"\t#{configuration}" \ "\t#{configuration}" \
"\n\n" \ "\n\n" \
"Finalize it manualy by running" \ "Finalize it manualy by running" \
......
...@@ -84,7 +84,7 @@ module Gitlab ...@@ -84,7 +84,7 @@ module Gitlab
FROM #{connection.quote_table_name(batch_table_name)} FROM #{connection.quote_table_name(batch_table_name)}
SQL SQL
migration_status = batch_max_value.nil? ? :finished : :active status_event = batch_max_value.nil? ? :finish : :execute
batch_max_value ||= batch_min_value batch_max_value ||= batch_min_value
migration = Gitlab::Database::BackgroundMigration::BatchedMigration.new( migration = Gitlab::Database::BackgroundMigration::BatchedMigration.new(
...@@ -98,7 +98,7 @@ module Gitlab ...@@ -98,7 +98,7 @@ module Gitlab
batch_class_name: batch_class_name, batch_class_name: batch_class_name,
batch_size: batch_size, batch_size: batch_size,
sub_batch_size: sub_batch_size, sub_batch_size: sub_batch_size,
status: migration_status status_event: status_event
) )
# Below `BatchedMigration` attributes were introduced after the # Below `BatchedMigration` attributes were introduced after the
......
...@@ -80,8 +80,8 @@ namespace :gitlab do ...@@ -80,8 +80,8 @@ namespace :gitlab do
def display_migration_status(database_name, connection) def display_migration_status(database_name, connection)
Gitlab::Database::SharedModel.using_connection(connection) do Gitlab::Database::SharedModel.using_connection(connection) do
statuses = Gitlab::Database::BackgroundMigration::BatchedMigration.statuses valid_status = Gitlab::Database::BackgroundMigration::BatchedMigration.valid_status
max_status_length = statuses.keys.map(&:length).max max_status_length = valid_status.map(&:length).max
format_string = "%-#{max_status_length}s | %s\n" format_string = "%-#{max_status_length}s | %s\n"
puts "Database: #{database_name}\n" puts "Database: #{database_name}\n"
...@@ -94,7 +94,7 @@ namespace :gitlab do ...@@ -94,7 +94,7 @@ namespace :gitlab do
migration.job_arguments.to_json migration.job_arguments.to_json
].join(',') ].join(',')
printf(format_string, migration.status, identification_fields) printf(format_string, migration.status_name, identification_fields)
end end
end end
end end
......
...@@ -340,7 +340,7 @@ namespace :gitlab do ...@@ -340,7 +340,7 @@ namespace :gitlab do
desc 'Run all pending batched migrations' desc 'Run all pending batched migrations'
task execute_batched_migrations: :environment do task execute_batched_migrations: :environment do
Gitlab::Database::BackgroundMigration::BatchedMigration.active.queue_order.each do |migration| Gitlab::Database::BackgroundMigration::BatchedMigration.with_status(:active).queue_order.each do |migration|
Gitlab::AppLogger.info("Executing batched migration #{migration.id} inline") Gitlab::AppLogger.info("Executing batched migration #{migration.id} inline")
Gitlab::Database::BackgroundMigration::BatchedMigrationRunner.new.run_entire_migration(migration) Gitlab::Database::BackgroundMigration::BatchedMigrationRunner.new.run_entire_migration(migration)
end end
......
...@@ -13,12 +13,24 @@ FactoryBot.define do ...@@ -13,12 +13,24 @@ FactoryBot.define do
total_tuple_count { 10_000 } total_tuple_count { 10_000 }
pause_ms { 100 } pause_ms { 100 }
trait :finished do trait(:paused) do
status { :finished } status { 0 }
end end
trait :failed do trait(:active) do
status { :failed } status { 1 }
end
trait(:finished) do
status { 3 }
end
trait(:failed) do
status { 4 }
end
trait(:finalizing) do
status { 5 }
end end
end end
end end
...@@ -5,9 +5,9 @@ require 'spec_helper' ...@@ -5,9 +5,9 @@ require 'spec_helper'
RSpec.describe "Admin > Admin sees background migrations" do RSpec.describe "Admin > Admin sees background migrations" do
let_it_be(:admin) { create(:admin) } let_it_be(:admin) { create(:admin) }
let_it_be(:active_migration) { create(:batched_background_migration, table_name: 'active', status: :active) } let_it_be(:active_migration) { create(:batched_background_migration, :active, table_name: 'active') }
let_it_be(:failed_migration) { create(:batched_background_migration, table_name: 'failed', status: :failed, total_tuple_count: 100) } let_it_be(:failed_migration) { create(:batched_background_migration, :failed, table_name: 'failed', total_tuple_count: 100) }
let_it_be(:finished_migration) { create(:batched_background_migration, table_name: 'finished', status: :finished) } let_it_be(:finished_migration) { create(:batched_background_migration, :finished, table_name: 'finished') }
before_all do before_all do
create(:batched_background_migration_job, :failed, batched_migration: failed_migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) create(:batched_background_migration_job, :failed, batched_migration: failed_migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3)
...@@ -81,7 +81,7 @@ RSpec.describe "Admin > Admin sees background migrations" do ...@@ -81,7 +81,7 @@ RSpec.describe "Admin > Admin sees background migrations" do
expect(page).to have_content(failed_migration.job_class_name) expect(page).to have_content(failed_migration.job_class_name)
expect(page).to have_content(failed_migration.table_name) expect(page).to have_content(failed_migration.table_name)
expect(page).to have_content('0.00%') expect(page).to have_content('0.00%')
expect(page).to have_content(failed_migration.status.humanize) expect(page).to have_content(failed_migration.status_name.to_s)
click_button('Retry') click_button('Retry')
expect(page).not_to have_content(failed_migration.job_class_name) expect(page).not_to have_content(failed_migration.job_class_name)
...@@ -106,7 +106,7 @@ RSpec.describe "Admin > Admin sees background migrations" do ...@@ -106,7 +106,7 @@ RSpec.describe "Admin > Admin sees background migrations" do
expect(page).to have_content(finished_migration.job_class_name) expect(page).to have_content(finished_migration.job_class_name)
expect(page).to have_content(finished_migration.table_name) expect(page).to have_content(finished_migration.table_name)
expect(page).to have_content('100.00%') expect(page).to have_content('100.00%')
expect(page).to have_content(finished_migration.status.humanize) expect(page).to have_content(finished_migration.status_name.to_s)
end end
end end
end end
...@@ -6,7 +6,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do ...@@ -6,7 +6,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do
describe '#batched_migration_status_badge_variant' do describe '#batched_migration_status_badge_variant' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:status, :variant) do where(:status_name, :variant) do
:active | :info :active | :info
:paused | :warning :paused | :warning
:failed | :danger :failed | :danger
...@@ -16,7 +16,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do ...@@ -16,7 +16,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do
subject { helper.batched_migration_status_badge_variant(migration) } subject { helper.batched_migration_status_badge_variant(migration) }
with_them do with_them do
let(:migration) { build(:batched_background_migration, status: status) } let(:migration) { build(:batched_background_migration, status_name) }
it { is_expected.to eq(variant) } it { is_expected.to eq(variant) }
end end
...@@ -25,7 +25,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do ...@@ -25,7 +25,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do
describe '#batched_migration_progress' do describe '#batched_migration_progress' do
subject { helper.batched_migration_progress(migration, completed_rows) } subject { helper.batched_migration_progress(migration, completed_rows) }
let(:migration) { build(:batched_background_migration, status: :active, total_tuple_count: 100) } let(:migration) { build(:batched_background_migration, :active, total_tuple_count: 100) }
let(:completed_rows) { 25 } let(:completed_rows) { 25 }
it 'returns completion percentage' do it 'returns completion percentage' do
...@@ -33,7 +33,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do ...@@ -33,7 +33,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do
end end
context 'when migration is finished' do context 'when migration is finished' do
let(:migration) { build(:batched_background_migration, status: :finished, total_tuple_count: nil) } let(:migration) { build(:batched_background_migration, :finished, total_tuple_count: nil) }
it 'returns 100 percent' do it 'returns 100 percent' do
expect(subject).to eq(100) expect(subject).to eq(100)
...@@ -41,7 +41,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do ...@@ -41,7 +41,7 @@ RSpec.describe Admin::BackgroundMigrationsHelper do
end end
context 'when total_tuple_count is nil' do context 'when total_tuple_count is nil' do
let(:migration) { build(:batched_background_migration, status: :active, total_tuple_count: nil) } let(:migration) { build(:batched_background_migration, :active, total_tuple_count: nil) }
it 'returns nil' do it 'returns nil' do
expect(subject).to eq(nil) expect(subject).to eq(nil)
......
...@@ -130,13 +130,11 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -130,13 +130,11 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
describe 'scopes' do describe 'scopes' do
let_it_be(:fixed_time) { Time.new(2021, 04, 27, 10, 00, 00, 00) } let_it_be(:fixed_time) { Time.new(2021, 04, 27, 10, 00, 00, 00) }
let_it_be(:pending_job) { create(:batched_background_migration_job, :pending, updated_at: fixed_time) } let_it_be(:pending_job) { create(:batched_background_migration_job, :pending, created_at: fixed_time - 2.days, updated_at: fixed_time) }
let_it_be(:running_job) { create(:batched_background_migration_job, :running, updated_at: fixed_time) } let_it_be(:running_job) { create(:batched_background_migration_job, :running, created_at: fixed_time - 2.days, updated_at: fixed_time) }
let_it_be(:stuck_job) { create(:batched_background_migration_job, :pending, updated_at: fixed_time - described_class::STUCK_JOBS_TIMEOUT) } let_it_be(:stuck_job) { create(:batched_background_migration_job, :pending, created_at: fixed_time, updated_at: fixed_time - described_class::STUCK_JOBS_TIMEOUT) }
let_it_be(:failed_job) { create(:batched_background_migration_job, :failed, attempts: 1) } let_it_be(:failed_job) { create(:batched_background_migration_job, :failed, created_at: fixed_time, attempts: 1) }
let_it_be(:max_attempts_failed_job) { create(:batched_background_migration_job, :failed, created_at: fixed_time, attempts: described_class::MAX_ATTEMPTS) }
let!(:max_attempts_failed_job) { create(:batched_background_migration_job, :failed, attempts: described_class::MAX_ATTEMPTS) }
let!(:succeeded_job) { create(:batched_background_migration_job, :succeeded) }
before do before do
travel_to fixed_time travel_to fixed_time
...@@ -165,6 +163,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -165,6 +163,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
expect(described_class.retriable).to contain_exactly(failed_job, stuck_job) expect(described_class.retriable).to contain_exactly(failed_job, stuck_job)
end end
end end
describe '.created_since' do
it 'returns jobs since a given time' do
expect(described_class.created_since(fixed_time)).to contain_exactly(stuck_job, failed_job, max_attempts_failed_job)
end
end
end end
describe 'delegated batched_migration attributes' do describe 'delegated batched_migration attributes' do
......
...@@ -86,6 +86,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -86,6 +86,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
end end
end end
context 'when the migration should stop' do
let(:migration) { create(:batched_background_migration, :active) }
let!(:job) { create(:batched_background_migration_job, :failed, batched_migration: migration) }
it 'changes the status to failure' do
expect(migration).to receive(:should_stop?).and_return(true)
expect(migration_wrapper).to receive(:perform).and_return(job)
expect { runner.run_migration_job(migration) }.to change { migration.status_name }.from(:active).to(:failed)
end
end
context 'when the migration has previous jobs' do context 'when the migration has previous jobs' do
let!(:event1) { create(:event) } let!(:event1) { create(:event) }
let!(:event2) { create(:event) } let!(:event2) { create(:event) }
...@@ -293,8 +306,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -293,8 +306,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
let!(:batched_migration) do let!(:batched_migration) do
create( create(
:batched_background_migration, :batched_background_migration, migration_status,
status: migration_status,
max_value: 8, max_value: 8,
batch_size: 2, batch_size: 2,
sub_batch_size: 1, sub_batch_size: 1,
...@@ -339,7 +351,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -339,7 +351,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
.with('CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments) .with('CopyColumnUsingBackgroundMigrationJob', table_name, column_name, job_arguments)
.and_return(batched_migration) .and_return(batched_migration)
expect(batched_migration).to receive(:finalizing!).and_call_original expect(batched_migration).to receive(:finalize!).and_call_original
expect do expect do
runner.finalize( runner.finalize(
...@@ -348,7 +360,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -348,7 +360,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
column_name, column_name,
job_arguments job_arguments
) )
end.to change { batched_migration.reload.status }.from('active').to('finished') end.to change { batched_migration.reload.status_name }.from(:active).to(:finished)
expect(batched_migration.batched_jobs).to all(be_succeeded) expect(batched_migration.batched_jobs).to all(be_succeeded)
...@@ -390,7 +402,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -390,7 +402,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
expect(Gitlab::AppLogger).to receive(:warn) expect(Gitlab::AppLogger).to receive(:warn)
.with("Batched background migration for the given configuration is already finished: #{configuration}") .with("Batched background migration for the given configuration is already finished: #{configuration}")
expect(batched_migration).not_to receive(:finalizing!) expect(batched_migration).not_to receive(:finalize!)
runner.finalize( runner.finalize(
batched_migration.job_class_name, batched_migration.job_class_name,
...@@ -417,7 +429,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -417,7 +429,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
expect(Gitlab::AppLogger).to receive(:warn) expect(Gitlab::AppLogger).to receive(:warn)
.with("Could not find batched background migration for the given configuration: #{configuration}") .with("Could not find batched background migration for the given configuration: #{configuration}")
expect(batched_migration).not_to receive(:finalizing!) expect(batched_migration).not_to receive(:finalize!)
runner.finalize( runner.finalize(
batched_migration.job_class_name, batched_migration.job_class_name,
......
...@@ -27,28 +27,46 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -27,28 +27,46 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
it { is_expected.to validate_uniqueness_of(:job_arguments).scoped_to(:job_class_name, :table_name, :column_name) } it { is_expected.to validate_uniqueness_of(:job_arguments).scoped_to(:job_class_name, :table_name, :column_name) }
context 'when there are failed jobs' do context 'when there are failed jobs' do
let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) } let(:batched_migration) { create(:batched_background_migration, :active, total_tuple_count: 100) }
let!(:batched_job) { create(:batched_background_migration_job, :failed, batched_migration: batched_migration) } let!(:batched_job) { create(:batched_background_migration_job, :failed, batched_migration: batched_migration) }
it 'raises an exception' do it 'raises an exception' do
expect { batched_migration.finished! }.to raise_error(ActiveRecord::RecordInvalid) expect { batched_migration.finish! }.to raise_error(StateMachines::InvalidTransition)
expect(batched_migration.reload.status).to eql 'active' expect(batched_migration.reload.status_name).to be :active
end end
end end
context 'when the jobs are completed' do context 'when the jobs are completed' do
let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) } let(:batched_migration) { create(:batched_background_migration, :active, total_tuple_count: 100) }
let!(:batched_job) { create(:batched_background_migration_job, :succeeded, batched_migration: batched_migration) } let!(:batched_job) { create(:batched_background_migration_job, :succeeded, batched_migration: batched_migration) }
it 'finishes the migration' do it 'finishes the migration' do
batched_migration.finished! batched_migration.finish!
expect(batched_migration.status).to eql 'finished' expect(batched_migration.status_name).to be :finished
end end
end end
end end
describe 'state machine' do
context 'when a migration is executed' do
let!(:batched_migration) { create(:batched_background_migration) }
it 'updates the started_at' do
expect { batched_migration.execute! }.to change(batched_migration, :started_at).from(nil).to(Time)
end
end
end
describe '.valid_status' do
valid_status = [:paused, :active, :finished, :failed, :finalizing]
it 'returns valid status' do
expect(described_class.valid_status).to eq(valid_status)
end
end
describe '.queue_order' do describe '.queue_order' do
let!(:migration1) { create(:batched_background_migration) } let!(:migration1) { create(:batched_background_migration) }
let!(:migration2) { create(:batched_background_migration) } let!(:migration2) { create(:batched_background_migration) }
...@@ -287,7 +305,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -287,7 +305,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
it 'moves the status of the migration to active' do it 'moves the status of the migration to active' do
retry_failed_jobs retry_failed_jobs
expect(batched_migration.status).to eql 'active' expect(batched_migration.status_name).to be :active
end end
it 'changes the number of attempts to 0' do it 'changes the number of attempts to 0' do
...@@ -301,11 +319,62 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -301,11 +319,62 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
it 'moves the status of the migration to active' do it 'moves the status of the migration to active' do
retry_failed_jobs retry_failed_jobs
expect(batched_migration.status).to eql 'active' expect(batched_migration.status_name).to be :active
end end
end end
end end
describe '#should_stop?' do
subject(:should_stop?) { batched_migration.should_stop? }
let(:batched_migration) { create(:batched_background_migration, started_at: started_at) }
before do
stub_const('Gitlab::Database::BackgroundMigration::BatchedMigration::MINIMUM_JOBS', 1)
end
context 'when the started_at is nil' do
let(:started_at) { nil }
it { expect(should_stop?).to be_falsey }
end
context 'when the number of jobs is lesser than the MINIMUM_JOBS' do
let(:started_at) { Time.zone.now - 6.days }
before do
stub_const('Gitlab::Database::BackgroundMigration::BatchedMigration::MINIMUM_JOBS', 10)
stub_const('Gitlab::Database::BackgroundMigration::BatchedMigration::MAXIMUM_FAILED_RATIO', 0.70)
create_list(:batched_background_migration_job, 1, :succeeded, batched_migration: batched_migration)
create_list(:batched_background_migration_job, 3, :failed, batched_migration: batched_migration)
end
it { expect(should_stop?).to be_falsey }
end
context 'when the calculated value is greater than the threshold' do
let(:started_at) { Time.zone.now - 6.days }
before do
stub_const('Gitlab::Database::BackgroundMigration::BatchedMigration::MAXIMUM_FAILED_RATIO', 0.70)
create_list(:batched_background_migration_job, 1, :succeeded, batched_migration: batched_migration)
create_list(:batched_background_migration_job, 3, :failed, batched_migration: batched_migration)
end
it { expect(should_stop?).to be_truthy }
end
context 'when the calculated value is lesser than the threshold' do
let(:started_at) { Time.zone.now - 6.days }
before do
create_list(:batched_background_migration_job, 2, :succeeded, batched_migration: batched_migration)
end
it { expect(should_stop?).to be_falsey }
end
end
describe '#job_class_name=' do describe '#job_class_name=' do
it_behaves_like 'an attr_writer that assigns class names', :job_class_name it_behaves_like 'an attr_writer that assigns class names', :job_class_name
end end
......
...@@ -2096,7 +2096,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -2096,7 +2096,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
end end
let(:migration_relation) { Gitlab::Database::BackgroundMigration::BatchedMigration.active } let(:migration_relation) { Gitlab::Database::BackgroundMigration::BatchedMigration.with_status(:active) }
before do before do
model.initialize_conversion_of_integer_to_bigint(table, columns) model.initialize_conversion_of_integer_to_bigint(table, columns)
...@@ -2218,7 +2218,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -2218,7 +2218,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
subject(:ensure_batched_background_migration_is_finished) { model.ensure_batched_background_migration_is_finished(**configuration) } subject(:ensure_batched_background_migration_is_finished) { model.ensure_batched_background_migration_is_finished(**configuration) }
it 'raises an error when migration exists and is not marked as finished' do it 'raises an error when migration exists and is not marked as finished' do
create(:batched_background_migration, configuration.merge(status: :active)) create(:batched_background_migration, :active, configuration)
expect { ensure_batched_background_migration_is_finished } expect { ensure_batched_background_migration_is_finished }
.to raise_error "Expected batched background migration for the given configuration to be marked as 'finished', but it is 'active':" \ .to raise_error "Expected batched background migration for the given configuration to be marked as 'finished', but it is 'active':" \
...@@ -2234,7 +2234,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -2234,7 +2234,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
it 'does not raise error when migration exists and is marked as finished' do it 'does not raise error when migration exists and is marked as finished' do
create(:batched_background_migration, configuration.merge(status: :finished)) create(:batched_background_migration, :finished, configuration)
expect { ensure_batched_background_migration_is_finished } expect { ensure_batched_background_migration_is_finished }
.not_to raise_error .not_to raise_error
......
...@@ -75,7 +75,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d ...@@ -75,7 +75,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d
max_batch_size: 10000, max_batch_size: 10000,
sub_batch_size: 10, sub_batch_size: 10,
job_arguments: %w[], job_arguments: %w[],
status: 'active', status_name: :active,
total_tuple_count: pgclass_info.cardinality_estimate) total_tuple_count: pgclass_info.cardinality_estimate)
end end
......
...@@ -10,7 +10,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do ...@@ -10,7 +10,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do
end end
describe 'POST #retry' do describe 'POST #retry' do
let(:migration) { create(:batched_background_migration, status: 'failed') } let(:migration) { create(:batched_background_migration, :failed) }
before do before do
create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3)
...@@ -37,11 +37,11 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do ...@@ -37,11 +37,11 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do
it 'retries the migration' do it 'retries the migration' do
retry_migration retry_migration
expect(migration.reload.status).to eql 'active' expect(migration.reload.status_name).to be :active
end end
context 'when the migration is not failed' do context 'when the migration is not failed' do
let(:migration) { create(:batched_background_migration, status: 'paused') } let(:migration) { create(:batched_background_migration, :paused) }
it 'keeps the same migration status' do it 'keeps the same migration status' do
expect { retry_migration }.not_to change { migration.reload.status } expect { retry_migration }.not_to change { migration.reload.status }
......
...@@ -710,7 +710,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do ...@@ -710,7 +710,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do
describe '#execute_batched_migrations' do describe '#execute_batched_migrations' do
subject { run_rake_task('gitlab:db:execute_batched_migrations') } subject { run_rake_task('gitlab:db:execute_batched_migrations') }
let(:migrations) { create_list(:batched_background_migration, 2) } let(:migrations) { create_list(:batched_background_migration, 2, :active) }
let(:runner) { instance_double('Gitlab::Database::BackgroundMigration::BatchedMigrationRunner') } let(:runner) { instance_double('Gitlab::Database::BackgroundMigration::BatchedMigrationRunner') }
before do before do
......
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