Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2018-06-08 23:06:08 +0300
committerJan Provaznik <jprovaznik@gitlab.com>2018-06-12 10:34:07 +0300
commitc3f499e7c8656fa09b333b88950c70f7377f95f9 (patch)
tree2d0a4175febeb24f5f613b72a4374b5a18131fef
parentfa1a75ae28b421c5d3bd926c2795a053e4c6af15 (diff)
Use upload ID instead of model ID in lease key
For FileUploaders it's possible that a model has many uploads and if lease key is created only from model id, it causes that the model's uploads can not be migrated in parallel because the exclusive lease key would be same for all uploads of the model.
-rw-r--r--app/uploaders/object_storage.rb13
-rw-r--r--changelogs/unreleased/47513-upload-migration-lease-key-is-incorrect-for-non-mounted-uploaders.yml5
-rw-r--r--spec/support/shared_examples/uploaders/object_storage_shared_examples.rb6
-rw-r--r--spec/uploaders/object_storage_spec.rb12
4 files changed, 28 insertions, 8 deletions
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index 23b3dcf84c0..d8ec5a81968 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -298,6 +298,15 @@ module ObjectStorage
super
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 = self.is_a?(RecordsUploads::Concern) && upload ? upload : model
+
+ "object_storage_migrate:#{key_object.class}:#{key_object.id}"
+ end
+
private
def schedule_background_upload?
@@ -364,10 +373,6 @@ module ObjectStorage
end
end
- def exclusive_lease_key
- "object_storage_migrate:#{model.class}:#{model.id}"
- end
-
def with_exclusive_lease
uuid = Gitlab::ExclusiveLease.new(exclusive_lease_key, timeout: 1.hour.to_i).try_obtain
raise 'exclusive lease already taken' unless uuid
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 6352f1527cd..12f7d3ed92a 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 0bc5b6751b3..902fc0bc030 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('exclusive lease already taken')
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