Merge branch '47513-upload-migration-lease-key-is-incorrect-for-non-mounted-uploaders' into 'master'

Resolve "Upload migration lease key is incorrect for non-mounted uploaders"

Closes #47513

See merge request gitlab-org/gitlab-ce!19600
This commit is contained in:
Sean McGivern 2018-06-13 09:45:34 +00:00
commit a66af9b121
4 changed files with 32 additions and 8 deletions

View file

@ -73,6 +73,15 @@ module ObjectStorage
upload.id)
end
def exclusive_lease_key
# For FileUploaders, model may have many uploaders. In that case
# we want to use exclusive key per upload, not per model to allow
# parallel migration
key_object = upload || model
"object_storage_migrate:#{key_object.class}:#{key_object.id}"
end
private
def current_upload_satisfies?(paths, model)
@ -316,6 +325,10 @@ module ObjectStorage
super
end
def exclusive_lease_key
"object_storage_migrate:#{model.class}:#{model.id}"
end
private
def schedule_background_upload?
@ -382,10 +395,6 @@ module ObjectStorage
end
end
def exclusive_lease_key
"object_storage_migrate:#{model.class}:#{model.id}"
end
def with_exclusive_lease
lease_key = exclusive_lease_key
uuid = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.hour.to_i).try_obtain

View file

@ -0,0 +1,5 @@
---
title: Use upload ID for creating lease key for file uploaders.
merge_request:
author:
type: fixed

View file

@ -76,10 +76,8 @@ shared_examples "migrates" do |to_store:, from_store: nil|
end
context 'when migrate! is occupied by another process' do
let(:exclusive_lease_key) { "object_storage_migrate:#{subject.model.class}:#{subject.model.id}" }
before do
@uuid = Gitlab::ExclusiveLease.new(exclusive_lease_key, timeout: 1.hour.to_i).try_obtain
@uuid = Gitlab::ExclusiveLease.new(subject.exclusive_lease_key, timeout: 1.hour.to_i).try_obtain
end
it 'does not execute migrate!' do
@ -95,7 +93,7 @@ shared_examples "migrates" do |to_store:, from_store: nil|
end
after do
Gitlab::ExclusiveLease.cancel(exclusive_lease_key, @uuid)
Gitlab::ExclusiveLease.cancel(subject.exclusive_lease_key, @uuid)
end
end

View file

@ -332,6 +332,18 @@ describe ObjectStorage do
expect { uploader.use_file }.to raise_error(ObjectStorage::ExclusiveLeaseTaken)
end
end
it 'can still migrate other files of the same model' do
uploader2 = uploader_class.new(object, :file)
uploader2.upload = create(:upload)
uploader.upload = create(:upload)
when_file_is_in_use do
expect(uploader2).to receive(:unsafe_migrate!)
uploader2.migrate!(described_class::Store::REMOTE)
end
end
end
describe '#fog_credentials' do