Commit 003bd3f5 authored by Simon Tomlinson's avatar Simon Tomlinson Committed by Tiger Watson

Test sampled jobs from background migrations

Add a method to randomly sample and run pending traditional background
migrations for a given duration. This will be called by an injected
migration during migration testing, as a first step to testing
background migrations.
parent f9caa6a1
......@@ -50,34 +50,41 @@ module Gitlab
Gitlab::Database::SharedModel.using_connection(connection, &block)
end
def steal(steal_class, retry_dead_jobs: false)
with_shared_connection do
def pending_jobs(include_dead_jobs: false)
Enumerator.new do |y|
queues = [
Sidekiq::ScheduledSet.new,
Sidekiq::Queue.new(self.queue)
]
if retry_dead_jobs
if include_dead_jobs
queues << Sidekiq::RetrySet.new
queues << Sidekiq::DeadSet.new
end
queues.each do |queue|
queue.each do |job|
migration_class, migration_args = job.args
y << job if job.klass == worker_class.name
end
end
end
end
def steal(steal_class, retry_dead_jobs: false)
with_shared_connection do
pending_jobs(include_dead_jobs: retry_dead_jobs).each do |job|
migration_class, migration_args = job.args
next unless job.klass == worker_class.name
next unless migration_class == steal_class
next if block_given? && !(yield job)
next unless migration_class == steal_class
next if block_given? && !(yield job)
begin
perform(migration_class, migration_args) if job.delete
rescue Exception # rubocop:disable Lint/RescueException
worker_class # enqueue this migration again
.perform_async(migration_class, migration_args)
begin
perform(migration_class, migration_args) if job.delete
rescue Exception # rubocop:disable Lint/RescueException
worker_class # enqueue this migration again
.perform_async(migration_class, migration_args)
raise
end
raise
end
end
end
......
# frozen_string_literal: true
module Gitlab
module Database
module Migrations
class TestBackgroundRunner
# TODO - build a rake task to call this method, and support it in the gitlab-com-database-testing project.
# Until then, we will inject a migration with a very high timestamp during database testing
# that calls this class to run jobs
# See https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing/-/issues/41 for details
def initialize
@job_coordinator = Gitlab::BackgroundMigration.coordinator_for_database(Gitlab::Database::MAIN_DATABASE_NAME)
end
def traditional_background_migrations
@job_coordinator.pending_jobs
end
def run_jobs(for_duration:)
jobs_to_run = traditional_background_migrations.group_by { |j| class_name_for_job(j) }
return if jobs_to_run.empty?
# without .to_f, we do integer division
# For example, 3.minutes / 2 == 1.minute whereas 3.minutes / 2.to_f == (1.minute + 30.seconds)
duration_per_migration_type = for_duration / jobs_to_run.count.to_f
jobs_to_run.each do |_migration_name, jobs|
run_until = duration_per_migration_type.from_now
jobs.shuffle.each do |j|
break if run_until <= Time.current
run_job(j)
end
end
end
private
def run_job(job)
Gitlab::BackgroundMigration.perform(job.args[0], job.args[1])
end
def class_name_for_job(job)
job.args[0]
end
end
end
end
end
......@@ -38,13 +38,67 @@ RSpec.describe Gitlab::BackgroundMigration::JobCoordinator do
end
end
describe '#pending_jobs' do
context 'when there are enqueued jobs' do
let(:queue) do
[
instance_double(Sidekiq::JobRecord, args: [1, 'queue'], klass: worker_class.name),
instance_double(Sidekiq::JobRecord, args: [2, 'queue'], klass: worker_class.name)
]
end
let(:queue_incorrect_job_class) do
[
instance_double(Sidekiq::JobRecord, args: [1, 'queue'], klass: 'SomeOtherClass')
]
end
let(:scheduled_set) do
[instance_double(Sidekiq::JobRecord, args: [3, 'scheduled'], klass: worker_class.name)]
end
let(:retry_set) do
[instance_double(Sidekiq::JobRecord, args: [4, 'retry'], klass: worker_class.name)]
end
let(:dead_set) do
[instance_double(Sidekiq::JobRecord, args: [5, 'dead'], klass: worker_class.name)]
end
before do
allow(Sidekiq::Queue).to receive(:new)
.with(coordinator.queue)
.and_return(queue + queue_incorrect_job_class)
allow(Sidekiq::ScheduledSet).to receive(:new).and_return(scheduled_set)
allow(Sidekiq::RetrySet).to receive(:new).and_return(retry_set)
allow(Sidekiq::DeadSet).to receive(:new).and_return(dead_set)
end
it 'does not include jobs for other workers' do
expect(coordinator.pending_jobs).not_to include(queue_incorrect_job_class.first)
end
context 'when not including dead jobs' do
it 'includes current and future jobs' do
expect(coordinator.pending_jobs(include_dead_jobs: false).to_a).to match_array(queue + scheduled_set)
end
end
context 'when including dead jobs' do
it 'includes current and future jobs, and also dead and retry jobs' do
expect(coordinator.pending_jobs(include_dead_jobs: true).to_a).to match_array(queue + scheduled_set + retry_set + dead_set)
end
end
end
end
describe '#steal' do
context 'when there are enqueued jobs present' do
let(:queue) do
[
double(args: ['Foo', [10, 20]], klass: worker_class.name),
double(args: ['Bar', [20, 30]], klass: worker_class.name),
double(args: ['Foo', [20, 30]], klass: 'MergeWorker')
instance_double(Sidekiq::JobRecord, args: ['Foo', [10, 20]], klass: worker_class.name),
instance_double(Sidekiq::JobRecord, args: ['Bar', [20, 30]], klass: worker_class.name),
instance_double(Sidekiq::JobRecord, args: ['Foo', [20, 30]], klass: 'MergeWorker')
]
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::Migrations::TestBackgroundRunner, :redis do
include Gitlab::Database::Migrations::BackgroundMigrationHelpers
# In order to test the interaction between queueing sidekiq jobs and seeing those jobs in queues,
# we need to disable sidekiq's testing mode and actually send our jobs to redis
around do |ex|
Sidekiq::Testing.disable! { ex.run }
end
context 'without jobs to run' do
it 'returns immediately' do
runner = described_class.new
expect(runner).not_to receive(:run_job)
described_class.new.run_jobs(for_duration: 1.second)
end
end
context 'with jobs to run' do
let(:migration_name) { 'TestBackgroundMigration' }
before do
(1..5).each do |i|
migrate_in(i.minutes, migration_name, [i])
end
end
context 'finding pending background jobs' do
it 'finds all the migrations' do
expect(described_class.new.traditional_background_migrations.to_a.size).to eq(5)
end
end
context 'running migrations', :freeze_time do
def define_background_migration(name)
klass = Class.new do
# Can't simply def perform here as we won't have access to the block,
# similarly can't define_method(:perform, &block) here as it would change the block receiver
define_method(:perform) { |*args| yield(*args) }
end
stub_const("Gitlab::BackgroundMigration::#{name}", klass)
klass
end
def expect_migration_call_counts(migrations_to_calls)
migrations_to_calls.each do |migration, calls|
expect_next_instances_of(migration, calls) do |m|
expect(m).to receive(:perform).and_call_original
end
end
end
it 'runs the migration class correctly' do
calls = []
define_background_migration(migration_name) do |i|
calls << i
end
described_class.new.run_jobs(for_duration: 1.second) # Any time would work here as we do not advance time
expect(calls).to contain_exactly(1, 2, 3, 4, 5)
end
it 'runs the migration for a uniform amount of time' do
migration = define_background_migration(migration_name) do |i|
travel(1.minute)
end
expect_migration_call_counts(migration => 3)
described_class.new.run_jobs(for_duration: 3.minutes)
end
context 'with multiple migrations to run' do
let(:other_migration_name) { 'OtherBackgroundMigration' }
before do
(1..5).each do |i|
migrate_in(i.minutes, other_migration_name, [i])
end
end
it 'splits the time between migrations when all migrations use all their time' do
migration = define_background_migration(migration_name) do |i|
travel(1.minute)
end
other_migration = define_background_migration(other_migration_name) do |i|
travel(2.minutes)
end
expect_migration_call_counts(
migration => 2, # 1 minute jobs for 90 seconds, can finish the first and start the second
other_migration => 1 # 2 minute jobs for 90 seconds, past deadline after a single job
)
described_class.new.run_jobs(for_duration: 3.minutes)
end
it 'does not give leftover time to extra migrations' do
# This is currently implemented this way for simplicity, but it could make sense to change this behavior.
migration = define_background_migration(migration_name) do
travel(1.second)
end
other_migration = define_background_migration(other_migration_name) do
travel(1.minute)
end
expect_migration_call_counts(
migration => 5,
other_migration => 2
)
described_class.new.run_jobs(for_duration: 3.minutes)
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