From d63380fa93dff921c69f7aaa31ff004864e4db13 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 23 Jan 2019 03:31:57 +0100 Subject: [PATCH] Refactor ProjectMigrate and ProjectRollback workers Moved to HashedStorage namespace, and added them to the `:hashed_storage` queue namespace --- app/models/project.rb | 8 ++-- app/workers/all_queues.yml | 4 +- .../hashed_storage/project_migrate_worker.rb | 48 +++++++++++++++++++ .../hashed_storage/project_rollback_worker.rb | 47 ++++++++++++++++++ .../project_migrate_hashed_storage_worker.rb | 43 ----------------- .../project_rollback_hashed_storage_worker.rb | 42 ---------------- .../gitlab/hashed_storage/migrator_spec.rb | 12 ++--- spec/models/project_spec.rb | 16 +++---- .../project_migrate_worker_spec.rb} | 2 +- .../project_rollback_worker_spec.rb} | 2 +- 10 files changed, 117 insertions(+), 107 deletions(-) create mode 100644 app/workers/hashed_storage/project_migrate_worker.rb create mode 100644 app/workers/hashed_storage/project_rollback_worker.rb delete mode 100644 app/workers/project_migrate_hashed_storage_worker.rb delete mode 100644 app/workers/project_rollback_hashed_storage_worker.rb rename spec/workers/{project_migrate_hashed_storage_worker_spec.rb => hashed_storage/project_migrate_worker_spec.rb} (94%) rename spec/workers/{project_rollback_hashed_storage_worker_spec.rb => hashed_storage/project_rollback_worker_spec.rb} (94%) diff --git a/app/models/project.rb b/app/models/project.rb index 400e86123f7..00592c108db 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1970,9 +1970,9 @@ class Project < ActiveRecord::Base return unless storage_upgradable? if git_transfer_in_progress? - ProjectMigrateHashedStorageWorker.perform_in(Gitlab::ReferenceCounter::REFERENCE_EXPIRE_TIME, id) + HashedStorage::ProjectMigrateWorker.perform_in(Gitlab::ReferenceCounter::REFERENCE_EXPIRE_TIME, id) else - ProjectMigrateHashedStorageWorker.perform_async(id) + HashedStorage::ProjectMigrateWorker.perform_async(id) end end @@ -1980,9 +1980,9 @@ class Project < ActiveRecord::Base return if legacy_storage? if git_transfer_in_progress? - ProjectRollbackHashedStorageWorker.perform_in(Gitlab::ReferenceCounter::REFERENCE_EXPIRE_TIME, id) + HashedStorage::ProjectRollbackWorker.perform_in(Gitlab::ReferenceCounter::REFERENCE_EXPIRE_TIME, id) else - ProjectRollbackHashedStorageWorker.perform_async(id) + HashedStorage::ProjectRollbackWorker.perform_async(id) end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 82351c1334f..a5dce221883 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -47,6 +47,8 @@ - github_importer:github_import_stage_import_repository - hashed_storage:hashed_storage_migrator +- hashed_storage:hashed_storage_project_migrate +- hashed_storage:hashed_storage_project_rollback - mail_scheduler:mail_scheduler_issue_due - mail_scheduler:mail_scheduler_notification_service @@ -126,8 +128,6 @@ - project_cache - project_destroy - project_export -- project_migrate_hashed_storage -- project_rollback_hashed_storage - project_service - propagate_service_template - reactive_caching diff --git a/app/workers/hashed_storage/project_migrate_worker.rb b/app/workers/hashed_storage/project_migrate_worker.rb new file mode 100644 index 00000000000..05bf72f519a --- /dev/null +++ b/app/workers/hashed_storage/project_migrate_worker.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module HashedStorage + class ProjectMigrateWorker + include ApplicationWorker + + LEASE_TIMEOUT = 30.seconds.to_i + LEASE_KEY_SEGMENT = 'project_migrate_hashed_storage_worker'.freeze + + queue_namespace :hashed_storage + + # rubocop: disable CodeReuse/ActiveRecord + def perform(project_id, old_disk_path = nil) + uuid = lease_for(project_id).try_obtain + + if uuid + project = Project.find_by(id: project_id) + return if project.nil? || project.pending_delete? + + old_disk_path ||= project.disk_path + + ::Projects::HashedStorage::MigrationService.new(project, old_disk_path, logger: logger).execute + else + return false + end + + ensure + cancel_lease_for(project_id, uuid) if uuid + end + + # rubocop: enable CodeReuse/ActiveRecord + + def lease_for(project_id) + Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT) + end + + private + + def lease_key(project_id) + # we share the same lease key for both migration and rollback so they don't run simultaneously + "#{LEASE_KEY_SEGMENT}:#{project_id}" + end + + def cancel_lease_for(project_id, uuid) + Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid) + end + end +end diff --git a/app/workers/hashed_storage/project_rollback_worker.rb b/app/workers/hashed_storage/project_rollback_worker.rb new file mode 100644 index 00000000000..ace9fea98a6 --- /dev/null +++ b/app/workers/hashed_storage/project_rollback_worker.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module HashedStorage + class ProjectRollbackWorker + include ApplicationWorker + + LEASE_TIMEOUT = 30.seconds.to_i + + queue_namespace :hashed_storage + + # rubocop: disable CodeReuse/ActiveRecord + def perform(project_id, old_disk_path = nil) + uuid = lease_for(project_id).try_obtain + + if uuid + project = Project.find_by(id: project_id) + return if project.nil? || project.pending_delete? + + old_disk_path ||= project.disk_path + + ::Projects::HashedStorage::RollbackService.new(project, old_disk_path, logger: logger).execute + else + return false + end + + ensure + cancel_lease_for(project_id, uuid) if uuid + end + + # rubocop: enable CodeReuse/ActiveRecord + + def lease_for(project_id) + Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT) + end + + private + + def lease_key(project_id) + # we share the same lease key for both migration and rollback so they don't run simultaneously + "#{ProjectMigrateWorker::LEASE_KEY_SEGMENT}:#{project_id}" + end + + def cancel_lease_for(project_id, uuid) + Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid) + end + end +end diff --git a/app/workers/project_migrate_hashed_storage_worker.rb b/app/workers/project_migrate_hashed_storage_worker.rb deleted file mode 100644 index 1c8f313e6e9..00000000000 --- a/app/workers/project_migrate_hashed_storage_worker.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -class ProjectMigrateHashedStorageWorker - include ApplicationWorker - - LEASE_TIMEOUT = 30.seconds.to_i - LEASE_KEY_SEGMENT = 'project_migrate_hashed_storage_worker'.freeze - - # rubocop: disable CodeReuse/ActiveRecord - def perform(project_id, old_disk_path = nil) - uuid = lease_for(project_id).try_obtain - - if uuid - project = Project.find_by(id: project_id) - return if project.nil? || project.pending_delete? - - old_disk_path ||= project.disk_path - - ::Projects::HashedStorage::MigrationService.new(project, old_disk_path, logger: logger).execute - else - return false - end - - ensure - cancel_lease_for(project_id, uuid) if uuid - end - # rubocop: enable CodeReuse/ActiveRecord - - def lease_for(project_id) - Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT) - end - - private - - def lease_key(project_id) - # we share the same lease key for both migration and rollback so they don't run simultaneously - "#{LEASE_KEY_SEGMENT}:#{project_id}" - end - - def cancel_lease_for(project_id, uuid) - Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid) - end -end diff --git a/app/workers/project_rollback_hashed_storage_worker.rb b/app/workers/project_rollback_hashed_storage_worker.rb deleted file mode 100644 index 38faffd2bfa..00000000000 --- a/app/workers/project_rollback_hashed_storage_worker.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -class ProjectRollbackHashedStorageWorker - include ApplicationWorker - - LEASE_TIMEOUT = 30.seconds.to_i - - # rubocop: disable CodeReuse/ActiveRecord - def perform(project_id, old_disk_path = nil) - uuid = lease_for(project_id).try_obtain - - if uuid - project = Project.find_by(id: project_id) - return if project.nil? || project.pending_delete? - - old_disk_path ||= project.disk_path - - ::Projects::HashedStorage::RollbackService.new(project, old_disk_path, logger: logger).execute - else - return false - end - - ensure - cancel_lease_for(project_id, uuid) if uuid - end - # rubocop: enable CodeReuse/ActiveRecord - - def lease_for(project_id) - Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT) - end - - private - - def lease_key(project_id) - # we share the same lease key for both migration and rollback so they don't run simultaneously - "#{ProjectMigrateHashedStorageWorker::LEASE_KEY_SEGMENT}:#{project_id}" - end - - def cancel_lease_for(project_id, uuid) - Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid) - end -end diff --git a/spec/lib/gitlab/hashed_storage/migrator_spec.rb b/spec/lib/gitlab/hashed_storage/migrator_spec.rb index 7c2582bb27a..000913f32bb 100644 --- a/spec/lib/gitlab/hashed_storage/migrator_spec.rb +++ b/spec/lib/gitlab/hashed_storage/migrator_spec.rb @@ -13,9 +13,9 @@ describe Gitlab::HashedStorage::Migrator do let(:projects) { create_list(:project, 2, :legacy_storage) } let(:ids) { projects.map(&:id) } - it 'enqueue jobs to ProjectMigrateHashedStorageWorker' do + it 'enqueue jobs to HashedStorage::ProjectMigrateWorker' do Sidekiq::Testing.fake! do - expect { subject.bulk_migrate(start: ids.min, finish: ids.max) }.to change(ProjectMigrateHashedStorageWorker.jobs, :size).by(2) + expect { subject.bulk_migrate(start: ids.min, finish: ids.max) }.to change(HashedStorage::ProjectMigrateWorker.jobs, :size).by(2) end end @@ -48,7 +48,7 @@ describe Gitlab::HashedStorage::Migrator do it 'enqueues project migration job' do Sidekiq::Testing.fake! do - expect { subject.migrate(project) }.to change(ProjectMigrateHashedStorageWorker.jobs, :size).by(1) + expect { subject.migrate(project) }.to change(HashedStorage::ProjectMigrateWorker.jobs, :size).by(1) end end @@ -79,7 +79,7 @@ describe Gitlab::HashedStorage::Migrator do it 'doesnt enqueue any migration job' do Sidekiq::Testing.fake! do - expect { subject.migrate(project) }.not_to change(ProjectMigrateHashedStorageWorker.jobs, :size) + expect { subject.migrate(project) }.not_to change(HashedStorage::ProjectMigrateWorker.jobs, :size) end end @@ -94,7 +94,7 @@ describe Gitlab::HashedStorage::Migrator do it 'enqueues project rollback job' do Sidekiq::Testing.fake! do - expect { subject.rollback(project) }.to change(ProjectRollbackHashedStorageWorker.jobs, :size).by(1) + expect { subject.rollback(project) }.to change(HashedStorage::ProjectRollbackWorker.jobs, :size).by(1) end end @@ -125,7 +125,7 @@ describe Gitlab::HashedStorage::Migrator do it 'doesnt enqueue any rollback job' do Sidekiq::Testing.fake! do - expect { subject.rollback(project) }.not_to change(ProjectRollbackHashedStorageWorker.jobs, :size) + expect { subject.rollback(project) }.not_to change(HashedStorage::ProjectRollbackWorker.jobs, :size) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6dc89b352f3..b2392f9521f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3430,24 +3430,24 @@ describe Project do project.migrate_to_hashed_storage! end - it 'schedules ProjectMigrateHashedStorageWorker with delayed start when the project repo is in use' do + it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the project repo is in use' do Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: false)).increase - expect(ProjectMigrateHashedStorageWorker).to receive(:perform_in) + expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in) project.migrate_to_hashed_storage! end - it 'schedules ProjectMigrateHashedStorageWorker with delayed start when the wiki repo is in use' do + it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the wiki repo is in use' do Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: true)).increase - expect(ProjectMigrateHashedStorageWorker).to receive(:perform_in) + expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in) project.migrate_to_hashed_storage! end - it 'schedules ProjectMigrateHashedStorageWorker' do - expect(ProjectMigrateHashedStorageWorker).to receive(:perform_async).with(project.id) + it 'schedules HashedStorage::ProjectMigrateWorker' do + expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_async).with(project.id) project.migrate_to_hashed_storage! end @@ -3541,7 +3541,7 @@ describe Project do project = create(:project, storage_version: 1, skip_disk_validation: true) Sidekiq::Testing.fake! do - expect { project.migrate_to_hashed_storage! }.to change(ProjectMigrateHashedStorageWorker.jobs, :size).by(1) + expect { project.migrate_to_hashed_storage! }.to change(HashedStorage::ProjectMigrateWorker.jobs, :size).by(1) end end end @@ -3566,7 +3566,7 @@ describe Project do it 'enqueues a job' do Sidekiq::Testing.fake! do - expect { project.rollback_to_legacy_storage! }.to change(ProjectRollbackHashedStorageWorker.jobs, :size).by(1) + expect { project.rollback_to_legacy_storage! }.to change(HashedStorage::ProjectRollbackWorker.jobs, :size).by(1) end end end diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/hashed_storage/project_migrate_worker_spec.rb similarity index 94% rename from spec/workers/project_migrate_hashed_storage_worker_spec.rb rename to spec/workers/hashed_storage/project_migrate_worker_spec.rb index 333eb6a0569..340e722aa7e 100644 --- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb +++ b/spec/workers/hashed_storage/project_migrate_worker_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do +describe HashedStorage::ProjectMigrateWorker, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers describe '#perform' do diff --git a/spec/workers/project_rollback_hashed_storage_worker_spec.rb b/spec/workers/hashed_storage/project_rollback_worker_spec.rb similarity index 94% rename from spec/workers/project_rollback_hashed_storage_worker_spec.rb rename to spec/workers/hashed_storage/project_rollback_worker_spec.rb index aed7493763d..d833553c0ec 100644 --- a/spec/workers/project_rollback_hashed_storage_worker_spec.rb +++ b/spec/workers/hashed_storage/project_rollback_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe ProjectRollbackHashedStorageWorker, :clean_gitlab_redis_shared_state do +describe HashedStorage::ProjectRollbackWorker, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers describe '#perform' do