Temporarily remove MR diffs removal migration
We will re-add this with a more efficient bulk scheduling method.
This commit is contained in:
parent
d285356e94
commit
15ec6a13eb
|
@ -1,70 +0,0 @@
|
|||
class EnqueueDeleteDiffFilesWorkers < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
class MergeRequestDiff < ActiveRecord::Base
|
||||
self.table_name = 'merge_request_diffs'
|
||||
|
||||
belongs_to :merge_request
|
||||
|
||||
include EachBatch
|
||||
end
|
||||
|
||||
DOWNTIME = false
|
||||
BATCH_SIZE = 1000
|
||||
MIGRATION = 'DeleteDiffFiles'
|
||||
DELAY_INTERVAL = 8.minutes
|
||||
TMP_INDEX = 'tmp_partial_diff_id_with_files_index'.freeze
|
||||
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
# We add temporary index, to make iteration over batches more performant.
|
||||
# Conditional here is to avoid the need of doing that in a separate
|
||||
# migration file to make this operation idempotent.
|
||||
#
|
||||
unless index_exists_by_name?(:merge_request_diffs, TMP_INDEX)
|
||||
add_concurrent_index(:merge_request_diffs, :id, where: "(state NOT IN ('without_files', 'empty'))", name: TMP_INDEX)
|
||||
end
|
||||
|
||||
|
||||
diffs_with_files = MergeRequestDiff.where.not(state: ['without_files', 'empty'])
|
||||
|
||||
# explain (analyze, buffers) example for the iteration:
|
||||
#
|
||||
# Index Only Scan using tmp_index_20013 on merge_request_diffs (cost=0.43..1630.19 rows=60567 width=4) (actual time=0.047..9.572 rows=56976 loops=1)
|
||||
# Index Cond: ((id >= 764586) AND (id < 835298))
|
||||
# Heap Fetches: 8
|
||||
# Buffers: shared hit=18188
|
||||
# Planning time: 0.752 ms
|
||||
# Execution time: 12.430 ms
|
||||
#
|
||||
diffs_with_files.each_batch(of: BATCH_SIZE) do |relation, outer_index|
|
||||
ids = relation.pluck(:id)
|
||||
|
||||
ids.each_with_index do |diff_id, inner_index|
|
||||
# This will give some space between batches of workers.
|
||||
interval = DELAY_INTERVAL * outer_index + inner_index.minutes
|
||||
|
||||
# A single `merge_request_diff` can be associated with way too many
|
||||
# `merge_request_diff_files`. It's better to avoid batching these and
|
||||
# schedule one at a time.
|
||||
#
|
||||
# Considering roughly 6M jobs, this should take ~30 days to process all
|
||||
# of them.
|
||||
#
|
||||
BackgroundMigrationWorker.perform_in(interval, MIGRATION, [diff_id])
|
||||
end
|
||||
end
|
||||
|
||||
# We remove temporary index, because it is not required during standard
|
||||
# operations and runtime.
|
||||
#
|
||||
remove_concurrent_index_by_name(:merge_request_diffs, TMP_INDEX)
|
||||
end
|
||||
|
||||
def down
|
||||
if index_exists_by_name?(:merge_request_diffs, TMP_INDEX)
|
||||
remove_concurrent_index_by_name(:merge_request_diffs, TMP_INDEX)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1,6 +1,6 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180619121030 do
|
||||
describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180626125654 do
|
||||
describe '#perform' do
|
||||
context 'when diff files can be deleted' do
|
||||
let(:merge_request) { create(:merge_request, :merged) }
|
||||
|
|
|
@ -1,48 +0,0 @@
|
|||
require 'spec_helper'
|
||||
require Rails.root.join('db', 'post_migrate', '20180619121030_enqueue_delete_diff_files_workers.rb')
|
||||
|
||||
describe EnqueueDeleteDiffFilesWorkers, :migration, :sidekiq do
|
||||
let(:merge_request_diffs) { table(:merge_request_diffs) }
|
||||
let(:merge_requests) { table(:merge_requests) }
|
||||
let(:namespaces) { table(:namespaces) }
|
||||
let(:projects) { table(:projects) }
|
||||
|
||||
before do
|
||||
stub_const("#{described_class.name}::BATCH_SIZE", 2)
|
||||
|
||||
namespaces.create!(id: 1, name: 'gitlab', path: 'gitlab')
|
||||
projects.create!(id: 1, namespace_id: 1, name: 'gitlab', path: 'gitlab')
|
||||
|
||||
merge_requests.create!(id: 1, target_project_id: 1, source_project_id: 1, target_branch: 'feature', source_branch: 'master', state: 'merged')
|
||||
|
||||
merge_request_diffs.create!(id: 1, merge_request_id: 1, state: 'collected')
|
||||
merge_request_diffs.create!(id: 2, merge_request_id: 1, state: 'without_files')
|
||||
merge_request_diffs.create!(id: 3, merge_request_id: 1, state: 'collected')
|
||||
merge_request_diffs.create!(id: 4, merge_request_id: 1, state: 'collected')
|
||||
merge_request_diffs.create!(id: 5, merge_request_id: 1, state: 'empty')
|
||||
merge_request_diffs.create!(id: 6, merge_request_id: 1, state: 'collected')
|
||||
|
||||
merge_requests.update(1, latest_merge_request_diff_id: 6)
|
||||
end
|
||||
|
||||
it 'correctly schedules diff file deletion workers' do
|
||||
Sidekiq::Testing.fake! do
|
||||
Timecop.freeze do
|
||||
migrate!
|
||||
|
||||
# 1st batch
|
||||
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(8.minutes, 1)
|
||||
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(9.minutes, 3)
|
||||
# 2nd batch
|
||||
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(16.minutes, 4)
|
||||
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(17.minutes, 6)
|
||||
expect(BackgroundMigrationWorker.jobs.size).to eq(4)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'migrates the data' do
|
||||
expect { migrate! }.to change { merge_request_diffs.where(state: 'without_files').count }
|
||||
.from(1).to(4)
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue