Commit bb06240e authored by Andreas Brandl's avatar Andreas Brandl

Merge branch '343047-add-ci-database-bg-migration-worker' into 'master'

Add the BG migration worker for the ci database

See merge request gitlab-org/gitlab!79064
parents 1683a70c 6dd41f59
...@@ -1947,6 +1947,15 @@ ...@@ -1947,6 +1947,15 @@
:weight: 1 :weight: 1
:idempotent: :idempotent:
:tags: [] :tags: []
- :name: background_migration_ci_database
:worker_name: BackgroundMigration::CiDatabaseWorker
:feature_category: :database
:has_external_dependencies:
:urgency: :throttled
:resource_boundary: :unknown
:weight: 1
:idempotent:
:tags: []
- :name: bulk_import - :name: bulk_import
:worker_name: BulkImportWorker :worker_name: BulkImportWorker
:feature_category: :importers :feature_category: :importers
......
# frozen_string_literal: true
module BackgroundMigration
class CiDatabaseWorker # rubocop:disable Scalability/IdempotentWorker
include SingleDatabaseWorker
def self.tracking_database
@tracking_database ||= Gitlab::Database::CI_DATABASE_NAME
end
end
end
...@@ -32,10 +32,6 @@ module BackgroundMigration ...@@ -32,10 +32,6 @@ module BackgroundMigration
def tracking_database def tracking_database
raise NotImplementedError, "#{self.name} does not implement #{__method__}" raise NotImplementedError, "#{self.name} does not implement #{__method__}"
end end
def unhealthy_metric_name
raise NotImplementedError, "#{self.name} does not implement #{__method__}"
end
end end
# Performs the background migration. # Performs the background migration.
...@@ -55,8 +51,12 @@ module BackgroundMigration ...@@ -55,8 +51,12 @@ module BackgroundMigration
private private
def tracking_database
self.class.tracking_database
end
def job_coordinator def job_coordinator
@job_coordinator ||= Gitlab::BackgroundMigration.coordinator_for_database(self.class.tracking_database) @job_coordinator ||= Gitlab::BackgroundMigration.coordinator_for_database(tracking_database)
end end
def perform_with_connection(class_name, arguments, lease_attempts) def perform_with_connection(class_name, arguments, lease_attempts)
...@@ -91,7 +91,7 @@ module BackgroundMigration ...@@ -91,7 +91,7 @@ module BackgroundMigration
healthy_db = healthy_database? healthy_db = healthy_database?
perform = lease_obtained && healthy_db perform = lease_obtained && healthy_db
database_unhealthy_counter.increment if lease_obtained && !healthy_db database_unhealthy_counter.increment(db_config_name: tracking_database) if lease_obtained && !healthy_db
# When the DB is unhealthy or the lease can't be obtained after several tries, # When the DB is unhealthy or the lease can't be obtained after several tries,
# then give up on the job and log a warning. Otherwise we could end up in # then give up on the job and log a warning. Otherwise we could end up in
...@@ -140,7 +140,7 @@ module BackgroundMigration ...@@ -140,7 +140,7 @@ module BackgroundMigration
def database_unhealthy_counter def database_unhealthy_counter
Gitlab::Metrics.counter( Gitlab::Metrics.counter(
self.class.unhealthy_metric_name, :background_migration_database_health_reschedules,
'The number of times a background migration is rescheduled because the database is unhealthy.' 'The number of times a background migration is rescheduled because the database is unhealthy.'
) )
end end
......
...@@ -6,8 +6,4 @@ class BackgroundMigrationWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -6,8 +6,4 @@ class BackgroundMigrationWorker # rubocop:disable Scalability/IdempotentWorker
def self.tracking_database def self.tracking_database
@tracking_database ||= Gitlab::BackgroundMigration::DEFAULT_TRACKING_DATABASE @tracking_database ||= Gitlab::BackgroundMigration::DEFAULT_TRACKING_DATABASE
end end
def self.unhealthy_metric_name
@unhealthy_metric_name ||= :background_migration_database_health_reschedules
end
end end
...@@ -55,6 +55,8 @@ ...@@ -55,6 +55,8 @@
- 3 - 3
- - background_migration - - background_migration
- 1 - 1
- - background_migration_ci_database
- 1
- - bulk_import - - bulk_import
- 1 - 1
- - bulk_imports_entity - - bulk_imports_entity
......
...@@ -10,28 +10,32 @@ module Gitlab ...@@ -10,28 +10,32 @@ module Gitlab
# Also provides a database connection to the correct tracking database. # Also provides a database connection to the correct tracking database.
class JobCoordinator # rubocop:disable Metrics/ClassLength class JobCoordinator # rubocop:disable Metrics/ClassLength
class << self class << self
def for_tracking_database(tracking_database)
worker_class = worker_for_tracking_database[tracking_database]
if worker_class.nil?
raise ArgumentError, "tracking_database must be one of [#{worker_for_tracking_database.keys.join(', ')}]"
end
new(worker_class)
end
private
def worker_classes def worker_classes
@worker_classes ||= [ @worker_classes ||= [
BackgroundMigrationWorker ::BackgroundMigrationWorker,
::BackgroundMigration::CiDatabaseWorker
].freeze ].freeze
end end
def worker_for_tracking_database def worker_for_tracking_database
@worker_for_tracking_database ||= worker_classes @worker_for_tracking_database ||= worker_classes
.select { |worker_class| Gitlab::Database.has_config?(worker_class.tracking_database) }
.index_by(&:tracking_database) .index_by(&:tracking_database)
.with_indifferent_access .with_indifferent_access
.freeze .freeze
end end
def for_tracking_database(tracking_database)
worker_class = worker_for_tracking_database[tracking_database]
if worker_class.nil?
raise ArgumentError, "tracking_database must be one of [#{worker_for_tracking_database.keys.join(', ')}]"
end
new(worker_class)
end
end end
attr_reader :worker_class attr_reader :worker_class
...@@ -146,7 +150,7 @@ module Gitlab ...@@ -146,7 +150,7 @@ module Gitlab
def connection def connection
@connection ||= Gitlab::Database @connection ||= Gitlab::Database
.database_base_models .database_base_models
.fetch(worker_class.tracking_database, Gitlab::Database::PRIMARY_DATABASE_NAME) .fetch(worker_class.tracking_database)
.connection .connection
end end
end end
......
...@@ -29,7 +29,7 @@ module Gitlab ...@@ -29,7 +29,7 @@ module Gitlab
# The worker classes aren't constants here, because that would force # The worker classes aren't constants here, because that would force
# Application Settings to be loaded earlier causing failures loading # Application Settings to be loaded earlier causing failures loading
# the environment in rake tasks # the environment in rake tasks
EXEMPT_WORKER_NAMES = ["BackgroundMigrationWorker", "Database::BatchedBackgroundMigrationWorker"].to_set EXEMPT_WORKER_NAMES = %w[BackgroundMigrationWorker BackgroundMigration::CiDatabaseWorker Database::BatchedBackgroundMigrationWorker].to_set
JOB_STATUS_KEY = 'size_limiter' JOB_STATUS_KEY = 'size_limiter'
class << self class << self
......
...@@ -299,7 +299,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do ...@@ -299,7 +299,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
before do before do
allow(Gitlab::BackgroundMigration).to receive(:coordinator_for_database) allow(Gitlab::BackgroundMigration).to receive(:coordinator_for_database)
.with('main').and_return(coordinator) .with(tracking_database).and_return(coordinator)
expect(coordinator).to receive(:migration_class_for) expect(coordinator).to receive(:migration_class_for)
.with(job_class_name).at_least(:once) { job_class } .with(job_class_name).at_least(:once) { job_class }
...@@ -403,7 +403,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do ...@@ -403,7 +403,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
end end
context 'when a specific coordinator is given' do context 'when a specific coordinator is given' do
let(:coordinator) { Gitlab::BackgroundMigration::JobCoordinator.for_tracking_database('main') } let(:coordinator) { Gitlab::BackgroundMigration::JobCoordinator.for_tracking_database(tracking_database) }
it 'uses that coordinator' do it 'uses that coordinator' do
expect(coordinator).to receive(:perform_in).with(10.minutes, 'Class', 'Hello', 'World').and_call_original expect(coordinator).to receive(:perform_in).with(10.minutes, 'Class', 'Hello', 'World').and_call_original
...@@ -438,6 +438,10 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do ...@@ -438,6 +438,10 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
it_behaves_like 'helpers that enqueue background migrations', BackgroundMigrationWorker, 'main' it_behaves_like 'helpers that enqueue background migrations', BackgroundMigrationWorker, 'main'
end end
context 'when the migration is running against the ci database', if: Gitlab::Database.has_config?(:ci) do
it_behaves_like 'helpers that enqueue background migrations', BackgroundMigration::CiDatabaseWorker, 'ci'
end
describe '#delete_job_tracking' do describe '#delete_job_tracking' do
let!(:job_class_name) { 'TestJob' } let!(:job_class_name) { 'TestJob' }
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'it runs background migration jobs' do |tracking_database, metric_name| RSpec.shared_examples 'it runs background migration jobs' do |tracking_database|
describe 'defining the job attributes' do describe 'defining the job attributes' do
it 'defines the data_consistency as always' do it 'defines the data_consistency as always' do
expect(described_class.get_data_consistency).to eq(:always) expect(described_class.get_data_consistency).to eq(:always)
...@@ -33,16 +33,6 @@ RSpec.shared_examples 'it runs background migration jobs' do |tracking_database, ...@@ -33,16 +33,6 @@ RSpec.shared_examples 'it runs background migration jobs' do |tracking_database,
end end
end end
describe '.unhealthy_metric_name' do
it 'does not raise an error' do
expect { described_class.unhealthy_metric_name }.not_to raise_error
end
it 'overrides the method to return the unhealthy metric name' do
expect(described_class.unhealthy_metric_name).to eq(metric_name)
end
end
describe '.minimum_interval' do describe '.minimum_interval' do
it 'returns 2 minutes' do it 'returns 2 minutes' do
expect(described_class.minimum_interval).to eq(2.minutes.to_i) expect(described_class.minimum_interval).to eq(2.minutes.to_i)
...@@ -189,11 +179,11 @@ RSpec.shared_examples 'it runs background migration jobs' do |tracking_database, ...@@ -189,11 +179,11 @@ RSpec.shared_examples 'it runs background migration jobs' do |tracking_database,
end end
it 'increments the unhealthy counter' do it 'increments the unhealthy counter' do
counter = Gitlab::Metrics.counter(metric_name, 'msg') counter = Gitlab::Metrics.counter(:background_migration_database_health_reschedules, 'msg')
expect(described_class).to receive(:perform_in) expect(described_class).to receive(:perform_in)
expect { worker.perform('Foo', [10, 20]) }.to change { counter.get }.by(1) expect { worker.perform('Foo', [10, 20]) }.to change { counter.get(db_config_name: tracking_database) }.by(1)
end end
context 'when lease_attempts is 0' do context 'when lease_attempts is 0' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe BackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, if: Gitlab::Database.has_config?(:ci) do
it_behaves_like 'it runs background migration jobs', 'ci'
end
...@@ -3,5 +3,5 @@ ...@@ -3,5 +3,5 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do RSpec.describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do
it_behaves_like 'it runs background migration jobs', 'main', :background_migration_database_health_reschedules it_behaves_like 'it runs background migration jobs', 'main'
end end
...@@ -135,6 +135,7 @@ RSpec.describe 'Every Sidekiq worker' do ...@@ -135,6 +135,7 @@ RSpec.describe 'Every Sidekiq worker' do
'AutoDevops::DisableWorker' => 3, 'AutoDevops::DisableWorker' => 3,
'AutoMergeProcessWorker' => 3, 'AutoMergeProcessWorker' => 3,
'BackgroundMigrationWorker' => 3, 'BackgroundMigrationWorker' => 3,
'BackgroundMigration::CiDatabaseWorker' => 3,
'BuildFinishedWorker' => 3, 'BuildFinishedWorker' => 3,
'BuildHooksWorker' => 3, 'BuildHooksWorker' => 3,
'BuildQueueWorker' => 3, 'BuildQueueWorker' => 3,
......
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