Codestyle changes and Added Exclusive Lease to hashed storage migration

3 files changed, 57 insertions, 15 deletions

diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb
index b58b6f57ed7..93f44110605 100644
--- a/app/services/projects/hashed_storage/migrate_attachments_service.rb
+++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb
@@ -33,7 +33,7 @@ module Projects
         end
 
         # Create hashed storage base path folder
-        FileUtils.mkdir_p(File.expand_path('..', new_path))
+        FileUtils.mkdir_p(File.dirname(new_path))
 
         FileUtils.mv(old_path, new_path)
         logger.info("Migrated project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})")
diff --git a/app/workers/project_migrate_hashed_storage_worker.rb b/app/workers/project_migrate_hashed_storage_worker.rb
index ca276d7801c..e3ecd6bc950 100644
--- a/app/workers/project_migrate_hashed_storage_worker.rb
+++ b/app/workers/project_migrate_hashed_storage_worker.rb
@@ -2,10 +2,34 @@ class ProjectMigrateHashedStorageWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
+  LEASE_TIMEOUT = 30.seconds.to_i
+
   def perform(project_id)
     project = Project.find_by(id: project_id)
     return if project.nil? || project.pending_delete?
 
-    ::Projects::HashedStorageMigrationService.new(project, logger).execute
+    uuid = try_obtain_lease_for(project_id)
+    if uuid
+      ::Projects::HashedStorageMigrationService.new(project, logger).execute
+    else
+      false
+    end
+  rescue => ex
+    cancel_lease_for(project_id, uuid)
+    raise ex
+  end
+
+  private
+
+  def try_obtain_lease_for(project_id)
+    Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT).try_obtain
+  end
+
+  def lease_key(project_id)
+    "project_migrate_hashed_storage_worker:#{project_id}"
+  end
+
+  def cancel_lease_for(project_id, uuid)
+    Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid)
   end
 end
diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/project_migrate_hashed_storage_worker_spec.rb
index f5226dee0ad..8cacdfa7173 100644
--- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb
+++ b/spec/workers/project_migrate_hashed_storage_worker_spec.rb
@@ -5,25 +5,43 @@ describe ProjectMigrateHashedStorageWorker do
     let(:project) { create(:project, :empty_repo) }
     let(:pending_delete_project) { create(:project, :empty_repo, pending_delete: true) }
 
-    it 'skips when project no longer exists' do
-      nonexistent_id = 999999999999
+    context 'when have exclusive lease' do
+      before do
+        allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true)
+      end
 
-      expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
-      subject.perform(nonexistent_id)
-    end
+      it 'skips when project no longer exists' do
+        nonexistent_id = 999999999999
+
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+        subject.perform(nonexistent_id)
+      end
+
+      it 'skips when project is pending delete' do
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
 
-    it 'skips when project is pending delete' do
-      expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+        subject.perform(pending_delete_project.id)
+      end
 
-      subject.perform(pending_delete_project.id)
+      it 'delegates removal to service class' do
+        service = double('service')
+        expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service)
+        expect(service).to receive(:execute)
+
+        subject.perform(project.id)
+      end
     end
 
-    it 'delegates removal to service class' do
-      service = double('service')
-      expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service)
-      expect(service).to receive(:execute)
+    context 'when dont have exclusive lease' do
+      before do
+        allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false)
+      end
+
+      it 'skips when dont have lease' do
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
 
-      subject.perform(project.id)
+        subject.perform(project.id)
+      end
     end
   end
 end