diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-06-13 09:45:34 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-06-13 09:45:34 +0000 |
commit | a66af9b121d3f03f46a689cb9bb0867628618974 (patch) | |
tree | d0538dd5c1b3b29365df7662c4e673a7e2afe405 | |
parent | 82c638d9f65db5d1beb6869aaeea01c43cac22d8 (diff) | |
parent | 3961407248c55f0524c8acfa154ace4ed33e087a (diff) | |
download | gitlab-ce-a66af9b121d3f03f46a689cb9bb0867628618974.tar.gz |
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
4 files changed, 32 insertions, 8 deletions
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index c6ea12cbe9d..b8ecfc4ee2b 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -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 diff --git a/changelogs/unreleased/47513-upload-migration-lease-key-is-incorrect-for-non-mounted-uploaders.yml b/changelogs/unreleased/47513-upload-migration-lease-key-is-incorrect-for-non-mounted-uploaders.yml new file mode 100644 index 00000000000..010c4e9bce7 --- /dev/null +++ b/changelogs/unreleased/47513-upload-migration-lease-key-is-incorrect-for-non-mounted-uploaders.yml @@ -0,0 +1,5 @@ +--- +title: Use upload ID for creating lease key for file uploaders. +merge_request: +author: +type: fixed diff --git a/spec/support/shared_examples/uploaders/object_storage_shared_examples.rb b/spec/support/shared_examples/uploaders/object_storage_shared_examples.rb index 1ecddc14d58..19800c6638f 100644 --- a/spec/support/shared_examples/uploaders/object_storage_shared_examples.rb +++ b/spec/support/shared_examples/uploaders/object_storage_shared_examples.rb @@ -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 diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index b497ddc3e67..c7f5694ff43 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -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 |