Commit fb8d04ad authored by Yannis Roussos's avatar Yannis Roussos

Merge branch '329511-do-not-duplicate-batched-background-migrations' into 'master'

Update queue_batched_background_migration not to duplicate migrations

See merge request gitlab-org/gitlab!62362
parents bd9b41b0 9e15c1cb
......@@ -16,6 +16,10 @@ module Gitlab
scope :queue_order, -> { order(id: :asc) }
scope :queued, -> { where(status: [:active, :paused]) }
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_arguments = ?", job_arguments.to_json) # rubocop:disable Rails/WhereEquals
end
enum status: {
paused: 0,
......
......@@ -137,6 +137,9 @@ module Gitlab
# class must be present in the Gitlab::BackgroundMigration module, and the batch class (if specified) must be
# present in the Gitlab::BackgroundMigration::BatchingStrategies module.
#
# If migration with same job_class_name, table_name, column_name, and job_aruments already exists, this helper
# will log an warning and not create a new one.
#
# job_class_name - The background migration job class as a string
# batch_table_name - The name of the table the migration will batch over
# batch_column_name - The name of the column the migration will batch over
......@@ -180,6 +183,13 @@ module Gitlab
sub_batch_size: SUB_BATCH_SIZE
)
if Gitlab::Database::BackgroundMigration::BatchedMigration.for_configuration(job_class_name, batch_table_name, batch_column_name, job_arguments).exists?
Gitlab::AppLogger.warn "Batched background migration not enqueued because it already exists: " \
"job_class_name: #{job_class_name}, table_name: #{batch_table_name}, column_name: #{batch_column_name}, " \
"job_arguments: #{job_arguments.inspect}"
return
end
job_interval = BATCH_MIN_DELAY if job_interval < BATCH_MIN_DELAY
batch_max_value ||= connection.select_value(<<~SQL)
......@@ -194,13 +204,13 @@ module Gitlab
job_class_name: job_class_name,
table_name: batch_table_name,
column_name: batch_column_name,
job_arguments: job_arguments,
interval: job_interval,
min_value: batch_min_value,
max_value: batch_max_value,
batch_class_name: batch_class_name,
batch_size: batch_size,
sub_batch_size: sub_batch_size,
job_arguments: job_arguments,
status: migration_status)
# This guard is necessary since #total_tuple_count was only introduced schema-wise,
......
......@@ -356,4 +356,29 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
subject
end
end
describe '.for_configuration' do
let!(:migration) do
create(
:batched_background_migration,
job_class_name: 'MyJobClass',
table_name: :projects,
column_name: :id,
job_arguments: [[:id], [:id_convert_to_bigint]]
)
end
before do
create(:batched_background_migration, job_class_name: 'OtherClass')
create(:batched_background_migration, table_name: 'other_table')
create(:batched_background_migration, column_name: 'other_column')
create(:batched_background_migration, job_arguments: %w[other arguments])
end
it 'finds the migration matching the given configuration parameters' do
actual = described_class.for_configuration('MyJobClass', :projects, :id, [[:id], [:id_convert_to_bigint]])
expect(actual).to contain_exactly(migration)
end
end
end
......@@ -269,6 +269,38 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
allow(Gitlab::Database::PgClass).to receive(:for_table).and_call_original
end
context 'when such migration already exists' do
it 'does not create duplicate migration' do
create(
:batched_background_migration,
job_class_name: 'MyJobClass',
table_name: :projects,
column_name: :id,
interval: 10.minutes,
min_value: 5,
max_value: 1005,
batch_class_name: 'MyBatchClass',
batch_size: 200,
sub_batch_size: 20,
job_arguments: [[:id], [:id_convert_to_bigint]]
)
expect do
model.queue_batched_background_migration(
'MyJobClass',
:projects,
:id,
[:id], [:id_convert_to_bigint],
job_interval: 5.minutes,
batch_min_value: 5,
batch_max_value: 1000,
batch_class_name: 'MyBatchClass',
batch_size: 100,
sub_batch_size: 10)
end.not_to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }
end
end
it 'creates the database record for the migration' do
expect(Gitlab::Database::PgClass).to receive(:for_table).with(:projects).and_return(pgclass_info)
......
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