From f4eb2ff725ae4f05679b2d1d41dbc3e033e7d3ac Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 11 Jul 2017 14:40:18 +0200 Subject: Implement build stage_id reference migration clean up --- ...10083355_build_stage_id_ref_migration_cleanup.rb | 21 +++++++++++++++++++++ db/schema.rb | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb diff --git a/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb b/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb new file mode 100644 index 00000000000..be8efb140d9 --- /dev/null +++ b/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb @@ -0,0 +1,21 @@ +require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') + +class BuildStageIdRefMigrationCleanup < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + ## + # `MigrateStageIdReferenceInBackground` background migration cleanup. + # + def up + Gitlab::BackgroundMigration + .steal(MigrateStageIdReferenceInBackground::MIGRATION) + end + + def down + # noop + end +end diff --git a/db/schema.rb b/db/schema.rb index 5264fc99557..44bc0109e66 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170707184244) do +ActiveRecord::Schema.define(version: 20170711145558) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- cgit v1.2.1 From bd6406c406e9894d1d3073978d42fcefa8631497 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 11 Jul 2017 15:19:44 +0200 Subject: Rename stage_id reference clean up migration --- ...10083355_build_stage_id_ref_migration_cleanup.rb | 21 --------------------- ...0710083355_clean_stage_id_reference_migration.rb | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 21 deletions(-) delete mode 100644 db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb create mode 100644 db/migrate/20170710083355_clean_stage_id_reference_migration.rb diff --git a/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb b/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb deleted file mode 100644 index be8efb140d9..00000000000 --- a/db/migrate/20170710083355_build_stage_id_ref_migration_cleanup.rb +++ /dev/null @@ -1,21 +0,0 @@ -require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') - -class BuildStageIdRefMigrationCleanup < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - ## - # `MigrateStageIdReferenceInBackground` background migration cleanup. - # - def up - Gitlab::BackgroundMigration - .steal(MigrateStageIdReferenceInBackground::MIGRATION) - end - - def down - # noop - end -end diff --git a/db/migrate/20170710083355_clean_stage_id_reference_migration.rb b/db/migrate/20170710083355_clean_stage_id_reference_migration.rb new file mode 100644 index 00000000000..f75cbb4a712 --- /dev/null +++ b/db/migrate/20170710083355_clean_stage_id_reference_migration.rb @@ -0,0 +1,21 @@ +require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') + +class CleanStageIdReferenceMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + ## + # `MigrateStageIdReferenceInBackground` background migration cleanup. + # + def up + Gitlab::BackgroundMigration + .steal(MigrateStageIdReferenceInBackground::MIGRATION) + end + + def down + # noop + end +end -- cgit v1.2.1 From fa3acb3bb662bef9d16a072f78d0048365a0f1dc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 11 Jul 2017 15:19:57 +0200 Subject: Add pending set of specs for stage_id cleanup migration --- .../clean_stage_id_reference_migration_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/migrations/clean_stage_id_reference_migration_spec.rb diff --git a/spec/migrations/clean_stage_id_reference_migration_spec.rb b/spec/migrations/clean_stage_id_reference_migration_spec.rb new file mode 100644 index 00000000000..17be549ddd3 --- /dev/null +++ b/spec/migrations/clean_stage_id_reference_migration_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20170710083355_clean_stage_id_reference_migration.rb') + +describe CleanStageIdReferenceMigration, :migration, :sidekiq do + context 'when there are enqueued background migrations' do + pending 'processes enqueued jobs synchronously' do + fail + end + end + + context 'when there are scheduled background migrations' do + pending 'immediately processes scheduled jobs' do + fail + end + end + + context 'when there are no background migrations pending' do + pending 'does nothing' do + fail + end + end +end -- cgit v1.2.1 From 5c3fd67075782b0cf1ef81254e81ae21b38a2012 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Jul 2017 10:31:22 +0200 Subject: Add specs for stage_id reference cleanup migration --- .../clean_stage_id_reference_migration_spec.rb | 29 ++++++++++++++-------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/spec/migrations/clean_stage_id_reference_migration_spec.rb b/spec/migrations/clean_stage_id_reference_migration_spec.rb index 17be549ddd3..0518c4de799 100644 --- a/spec/migrations/clean_stage_id_reference_migration_spec.rb +++ b/spec/migrations/clean_stage_id_reference_migration_spec.rb @@ -1,22 +1,29 @@ require 'spec_helper' require Rails.root.join('db', 'migrate', '20170710083355_clean_stage_id_reference_migration.rb') -describe CleanStageIdReferenceMigration, :migration, :sidekiq do - context 'when there are enqueued background migrations' do - pending 'processes enqueued jobs synchronously' do - fail - end - end +describe CleanStageIdReferenceMigration, :migration, :sidekiq, :redis do + let(:migration) { MigrateStageIdReferenceInBackground::MIGRATION } + + context 'when there are pending background migrations' do + it 'processes enqueued jobs synchronously' do + Sidekiq::Testing.disable! do + BackgroundMigrationWorker.perform_in(2.minutes, migration, [1]) + BackgroundMigrationWorker.perform_async(migration, [1]) - context 'when there are scheduled background migrations' do - pending 'immediately processes scheduled jobs' do - fail + expect(Gitlab::BackgroundMigration).to receive(:perform).twice + + migrate! + end end end context 'when there are no background migrations pending' do - pending 'does nothing' do - fail + it 'does nothing' do + Sidekiq::Testing.disable! do + expect(Gitlab::BackgroundMigration).not_to receive(:perform) + + migrate! + end end end end -- cgit v1.2.1 From 2930c0e3d090d5b33133160d847153a215ab059a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Jul 2017 10:43:12 +0200 Subject: Remove obsolete argument from bg migrations code --- lib/gitlab/background_migration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/background_migration.rb b/lib/gitlab/background_migration.rb index b0741b1fba7..d3f66877672 100644 --- a/lib/gitlab/background_migration.rb +++ b/lib/gitlab/background_migration.rb @@ -26,7 +26,7 @@ module Gitlab next unless migration_class == steal_class begin - perform(migration_class, migration_args, retries: 3) if job.delete + perform(migration_class, migration_args) if job.delete rescue Exception # rubocop:disable Lint/RescueException BackgroundMigrationWorker # enqueue this migration again .perform_async(migration_class, migration_args) -- cgit v1.2.1 From 73c7b968850b77dd2d740b494b4a98adb1222d41 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Jul 2017 11:51:47 +0200 Subject: Remove migration dependency from stage_id migration --- db/migrate/20170710083355_clean_stage_id_reference_migration.rb | 5 +---- spec/migrations/clean_stage_id_reference_migration_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/db/migrate/20170710083355_clean_stage_id_reference_migration.rb b/db/migrate/20170710083355_clean_stage_id_reference_migration.rb index f75cbb4a712..681203eaf40 100644 --- a/db/migrate/20170710083355_clean_stage_id_reference_migration.rb +++ b/db/migrate/20170710083355_clean_stage_id_reference_migration.rb @@ -1,5 +1,3 @@ -require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') - class CleanStageIdReferenceMigration < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers @@ -11,8 +9,7 @@ class CleanStageIdReferenceMigration < ActiveRecord::Migration # `MigrateStageIdReferenceInBackground` background migration cleanup. # def up - Gitlab::BackgroundMigration - .steal(MigrateStageIdReferenceInBackground::MIGRATION) + Gitlab::BackgroundMigration.steal('MigrateBuildStageIdReference') end def down diff --git a/spec/migrations/clean_stage_id_reference_migration_spec.rb b/spec/migrations/clean_stage_id_reference_migration_spec.rb index 0518c4de799..1b8d044ed61 100644 --- a/spec/migrations/clean_stage_id_reference_migration_spec.rb +++ b/spec/migrations/clean_stage_id_reference_migration_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' require Rails.root.join('db', 'migrate', '20170710083355_clean_stage_id_reference_migration.rb') +require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') describe CleanStageIdReferenceMigration, :migration, :sidekiq, :redis do let(:migration) { MigrateStageIdReferenceInBackground::MIGRATION } context 'when there are pending background migrations' do - it 'processes enqueued jobs synchronously' do + it 'processes pending jobs synchronously' do Sidekiq::Testing.disable! do BackgroundMigrationWorker.perform_in(2.minutes, migration, [1]) BackgroundMigrationWorker.perform_async(migration, [1]) -- cgit v1.2.1 From a65f64dfe645da893e92a061fd86437a55726873 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Jul 2017 11:55:43 +0200 Subject: Fix background migrations module specs --- spec/lib/gitlab/background_migration_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb index cfa59280139..4ad69aeba43 100644 --- a/spec/lib/gitlab/background_migration_spec.rb +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::BackgroundMigration do expect(queue[0]).to receive(:delete).and_return(true) expect(described_class).to receive(:perform) - .with('Foo', [10, 20], anything) + .with('Foo', [10, 20]) described_class.steal('Foo') end @@ -93,9 +93,9 @@ describe Gitlab::BackgroundMigration do it 'steals from the scheduled sets queue first' do Sidekiq::Testing.disable! do expect(described_class).to receive(:perform) - .with('Object', [1], anything).ordered + .with('Object', [1]).ordered expect(described_class).to receive(:perform) - .with('Object', [2], anything).ordered + .with('Object', [2]).ordered BackgroundMigrationWorker.perform_async('Object', [2]) BackgroundMigrationWorker.perform_in(10.minutes, 'Object', [1]) -- cgit v1.2.1 From a468c3a3179d7c7b84a0f7d1e6253238e40b9100 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Jul 2017 12:03:56 +0200 Subject: Fix database schema version number --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 44bc0109e66..623f22289ba 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170711145558) do +ActiveRecord::Schema.define(version: 20170710083355) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- cgit v1.2.1 From a6d1e92d98e71098c5a32999294bcdce6c7a092d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 19 Jul 2017 13:19:27 +0200 Subject: Isolate stage_id reference clean up migration This addreses a review remarks discussed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12785/diffs#note_35276344 --- spec/migrations/clean_stage_id_reference_migration_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/migrations/clean_stage_id_reference_migration_spec.rb b/spec/migrations/clean_stage_id_reference_migration_spec.rb index 1b8d044ed61..c2072f2672d 100644 --- a/spec/migrations/clean_stage_id_reference_migration_spec.rb +++ b/spec/migrations/clean_stage_id_reference_migration_spec.rb @@ -1,17 +1,17 @@ require 'spec_helper' require Rails.root.join('db', 'migrate', '20170710083355_clean_stage_id_reference_migration.rb') -require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') describe CleanStageIdReferenceMigration, :migration, :sidekiq, :redis do - let(:migration) { MigrateStageIdReferenceInBackground::MIGRATION } + let(:migration) { 'MigrateBuildStageIdReference' } context 'when there are pending background migrations' do it 'processes pending jobs synchronously' do Sidekiq::Testing.disable! do - BackgroundMigrationWorker.perform_in(2.minutes, migration, [1]) - BackgroundMigrationWorker.perform_async(migration, [1]) + BackgroundMigrationWorker.perform_in(2.minutes, migration, [1, 1]) + BackgroundMigrationWorker.perform_async(migration, [1, 1]) - expect(Gitlab::BackgroundMigration).to receive(:perform).twice + expect(Gitlab::BackgroundMigration) + .to receive(:perform).twice.and_call_original migrate! end -- cgit v1.2.1 From 841de9752fb35b7d1a701da8764729ba334c2da5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 20 Jul 2017 10:53:52 +0200 Subject: Fix background migration cleanup specs We need to use a spy because an `after` RSpec hook is also going to call the migration we want to test, so we need to use `have_received` expectation. See gitlab-org/gitlab-ce#35351 for more details. --- .../clean_stage_id_reference_migration_spec.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/spec/migrations/clean_stage_id_reference_migration_spec.rb b/spec/migrations/clean_stage_id_reference_migration_spec.rb index c2072f2672d..9a581df28a2 100644 --- a/spec/migrations/clean_stage_id_reference_migration_spec.rb +++ b/spec/migrations/clean_stage_id_reference_migration_spec.rb @@ -2,28 +2,32 @@ require 'spec_helper' require Rails.root.join('db', 'migrate', '20170710083355_clean_stage_id_reference_migration.rb') describe CleanStageIdReferenceMigration, :migration, :sidekiq, :redis do - let(:migration) { 'MigrateBuildStageIdReference' } + let(:migration_class) { 'MigrateBuildStageIdReference' } + let(:migration) { spy('migration') } + + before do + allow(Gitlab::BackgroundMigration.const_get(migration_class)) + .to receive(:new).and_return(migration) + end context 'when there are pending background migrations' do it 'processes pending jobs synchronously' do Sidekiq::Testing.disable! do - BackgroundMigrationWorker.perform_in(2.minutes, migration, [1, 1]) - BackgroundMigrationWorker.perform_async(migration, [1, 1]) - - expect(Gitlab::BackgroundMigration) - .to receive(:perform).twice.and_call_original + BackgroundMigrationWorker.perform_in(2.minutes, migration_class, [1, 1]) + BackgroundMigrationWorker.perform_async(migration_class, [1, 1]) migrate! + + expect(migration).to have_received(:perform).with(1, 1).twice end end end - context 'when there are no background migrations pending' do it 'does nothing' do Sidekiq::Testing.disable! do - expect(Gitlab::BackgroundMigration).not_to receive(:perform) - migrate! + + expect(migration).not_to have_received(:perform) end end end -- cgit v1.2.1