diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-06-13 11:30:35 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-06-13 11:30:35 +0300 |
commit | 8d9e0c0b8c419c024f67b58b4edc1efb2f99ca2e (patch) | |
tree | cb319cc3b53634c1389a87df0b90c3439c4f017d /app | |
parent | 4666ef683c75b5c053e579010926e03cdffa497f (diff) | |
parent | 3d42bab71ad293c99d029dfb4f0c9aa0378643d4 (diff) |
Merge branch '47408-migrateuploadsworker-is-doing-n-1-queries-on-migration' into 'master'
Resolve "`MigrateUploadsWorker` is doing N+1 queries on migration"
Closes #47408
See merge request gitlab-org/gitlab-ce!19547
Diffstat (limited to 'app')
-rw-r--r-- | app/uploaders/file_uploader.rb | 6 | ||||
-rw-r--r-- | app/uploaders/object_storage.rb | 29 | ||||
-rw-r--r-- | app/uploaders/records_uploads.rb | 2 |
3 files changed, 28 insertions, 9 deletions
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 133fdf6684d..36bc0a4575a 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -65,10 +65,10 @@ class FileUploader < GitlabUploader SecureRandom.hex end - def upload_paths(filename) + def upload_paths(identifier) [ - File.join(secret, filename), - File.join(base_dir(Store::REMOTE), secret, filename) + File.join(secret, identifier), + File.join(base_dir(Store::REMOTE), secret, identifier) ] end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 23b3dcf84c0..c6ea12cbe9d 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -10,6 +10,17 @@ module ObjectStorage UnknownStoreError = Class.new(StandardError) ObjectStorageUnavailable = Class.new(StandardError) + class ExclusiveLeaseTaken < StandardError + def initialize(lease_key) + @lease_key = lease_key + end + + def message + *lease_key_group, _ = *@lease_key.split(":") + "Exclusive lease for #{lease_key_group.join(':')} is already taken." + end + end + TMP_UPLOAD_PATH = 'tmp/uploads'.freeze module Store @@ -29,7 +40,7 @@ module ObjectStorage end def retrieve_from_store!(identifier) - paths = store_dirs.map { |store, path| File.join(path, identifier) } + paths = upload_paths(identifier) unless current_upload_satisfies?(paths, model) # the upload we already have isn't right, find the correct one @@ -261,7 +272,7 @@ module ObjectStorage end def delete_migrated_file(migrated_file) - migrated_file.delete if exists? + migrated_file.delete end def exists? @@ -279,6 +290,13 @@ module ObjectStorage } end + # Returns all the possible paths for an upload. + # the `upload.path` is a lookup parameter, and it may change + # depending on the `store` param. + def upload_paths(identifier) + store_dirs.map { |store, path| File.join(path, identifier) } + end + def cache!(new_file = sanitized_file) # We intercept ::UploadedFile which might be stored on remote storage # We use that for "accelerated" uploads, where we store result on remote storage @@ -369,12 +387,13 @@ module ObjectStorage 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 + lease_key = exclusive_lease_key + uuid = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.hour.to_i).try_obtain + raise ExclusiveLeaseTaken.new(lease_key) unless uuid yield uuid ensure - Gitlab::ExclusiveLease.cancel(exclusive_lease_key, uuid) + Gitlab::ExclusiveLease.cancel(lease_key, uuid) end # diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb index 89c74a78835..301f4681fcd 100644 --- a/app/uploaders/records_uploads.rb +++ b/app/uploaders/records_uploads.rb @@ -22,7 +22,7 @@ module RecordsUploads Upload.transaction do uploads.where(path: upload_path).delete_all - upload.destroy! if upload + upload.delete if upload self.upload = build_upload.tap(&:save!) end |