summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlessio Caiazza <acaiazza@gitlab.com>2018-07-03 11:36:09 +0000
committerAlessio Caiazza <acaiazza@gitlab.com>2018-07-03 13:40:10 +0200
commit1a7e070aa7a59aeb244e591038474f882c9fb76c (patch)
treee6c40ce21dcbfe132990d4895d392fc71da25ef7
parenta571725a6113c5a82b145e8d4714468d010457c0 (diff)
downloadgitlab-ce-1a7e070aa7a59aeb244e591038474f882c9fb76c.tar.gz
Merge branch 'temporarily-remove-mr-diffs-migration' into 'master'
Temporarily remove MR diffs removal migration See merge request gitlab-org/gitlab-ce!20330
-rw-r--r--db/post_migrate/20180619121030_enqueue_delete_diff_files_workers.rb70
-rw-r--r--spec/lib/gitlab/background_migration/delete_diff_files_spec.rb2
-rw-r--r--spec/migrations/enqueue_delete_diff_files_workers_spec.rb48
3 files changed, 1 insertions, 119 deletions
diff --git a/db/post_migrate/20180619121030_enqueue_delete_diff_files_workers.rb b/db/post_migrate/20180619121030_enqueue_delete_diff_files_workers.rb
deleted file mode 100644
index 5fb3d545624..00000000000
--- a/db/post_migrate/20180619121030_enqueue_delete_diff_files_workers.rb
+++ /dev/null
@@ -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
diff --git a/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb b/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
index a251ab323d8..1b3df7b20d4 100644
--- a/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
+++ b/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
@@ -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) }
diff --git a/spec/migrations/enqueue_delete_diff_files_workers_spec.rb b/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
deleted file mode 100644
index 686027822b8..00000000000
--- a/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
+++ /dev/null
@@ -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